Closed
Bug 1314147
Opened 8 years ago
Closed 7 years ago
Support the AOMedia Video 1 (AV1) codec format
Categories
(Core :: Audio/Video: Playback, defect, P3)
Core
Audio/Video: Playback
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: rillian, Assigned: rillian)
References
(Blocks 1 open bug)
Details
(Keywords: feature)
Attachments
(8 files, 5 obsolete files)
59 bytes,
text/x-review-board-request
|
froydnj
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
froydnj
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
froydnj
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
froydnj
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
kinetik
:
review+
froydnj
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
jya
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
jya
:
review+
|
Details |
59 bytes,
text/x-review-board-request
|
jya
:
review+
|
Details |
Tracking bug for supporting the Alliance for Open Media's av1 codec. https://aomedia.googlesource.com/aom/ http://aomedia.org/
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → giles
Updated•8 years ago
|
Priority: -- → P3
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 18•7 years ago
|
||
FYI: --enable-av1 builds fine on FreeBSD 12 (Clang 4.0 or GCC 6.3).
Assignee | ||
Comment 19•7 years ago
|
||
That's great to hear. Thanks for testing so quickly!
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 28•7 years ago
|
||
mozreview-review |
Comment on attachment 8857713 [details] Bug 1314147 - Port the libvpx mozbuild generator to aom. https://reviewboard.mozilla.org/r/129644/#review134722 ::: media/libaom/moz.build:12 (Diff revision 4) > +with Files('*'): > + BUG_COMPONENT = ('Core', 'Audio/Video') > + > +include('sources.mozbuild') > + > +if CONFIG['VPX_USE_YASM']: `VPX_USE_YASM` isn't defined for `--with-system-libvpx`. Maybe replace with `HAVE_YASM` + `INTEL_ARCHITECTURE` or adjust old-configure.in while keeping in mind `--with-system-aom` maybe added in future. ``` /usr/bin/clang -std=gnu99 -o aom_convolve_copy_sse2.o -fPIC -Wa,--noexecstack -include objdir/mozilla-config.h -DMOZILLA_CLIENT -Imedia/libaom/config/linux/x64/ -I. -Ithird_party/aom -c third_party/aom/aom_dsp/x86/aom_convolve_copy_sse2.asm clang: warning: argument unused during compilation: '-std=gnu99' [-Wunused-command-line-argument] clang: warning: argument unused during compilation: '-include objdir/mozilla-config.h' [-Wunused-command-line-argument] clang: warning: argument unused during compilation: '-D MOZILLA_CLIENT' [-Wunused-command-line-argument] third_party/aom/aom_dsp/x86/aom_convolve_copy_sse2.asm:2:17: error: unexpected token in argument list ; Copyright (c) 2016, Alliance for Open Media. All rights reserved ^ third_party/aom/aom_dsp/x86/aom_convolve_copy_sse2.asm:4:15: error: unexpected token in argument list ; This source code is subject to the terms of the BSD 2 Clause License and ^ [...] ```
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 42•7 years ago
|
||
With this patch set, I can play back https://people-mozilla.org/~rgiles/2017/rush_hour_444.av1.webm although the aspect ratio is wrong. Another file crashes with a simd alignment assert: https://people-mozilla.org/~rgiles/2017/sintel_trailer_2k_480p24.av1.webm Still, a happy milestone.
Updated•7 years ago
|
Alias: AV1
Has Regression Range: --- → irrelevant
Has STR: --- → irrelevant
Keywords: feature
OS: Unspecified → All
Hardware: Unspecified → All
Summary: Support the av1 video codec → Support the AOMedia Video 1 (AV1) codec format
Assignee | ||
Comment 43•7 years ago
|
||
(In reply to Jan Beich from comment #28) > `VPX_USE_YASM` isn't defined for `--with-system-libvpx`. Maybe replace with > `HAVE_YASM` + `INTEL_ARCHITECTURE` or adjust old-configure.in Thanks for testing. I'm happy to do this. Unfortunately VPX_USE_ASM is the easy part... VPX_AS_CONVERSION is fairly simple to redefine locally in the moz.build: it's defined on arm when GNU_AS is defined. Are you aware of any situations where GNU_AS wouldn't be available? The hard part is VPX_AS_FLAGS, which has a several bits to it, and also isn't defined with --with-system-libvpx. I'm not sure what to do about this case besides moving all that logic into the moz.configure side and making it available separately to both the libvpx and libaom moz.build files. That may have to wait until after this lands.
Assignee | ||
Comment 44•7 years ago
|
||
Actually, it looks like the USE_YASM setting here is dead code. I don't see a subsequent use. The ASFLAGS are still a problem though.
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8860177 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8860179 -
Attachment is obsolete: true
Assignee | ||
Updated•7 years ago
|
Attachment #8860181 -
Attachment is obsolete: true
Comment 55•7 years ago
|
||
mozreview-review |
Comment on attachment 8859732 [details] Bug 1314147 - Add AOMDecoder. https://reviewboard.mozilla.org/r/131728/#review139144 ::: dom/media/platforms/agnostic/AOMDecoder.cpp:29 (Diff revision 3) > +using namespace gfx; > +using namespace layers; > + > + > +static nsresult > +InitContext(aom_codec_ctx_t* aCtx, please use MediaResult so additional details can be returned and reported back to the user. ::: dom/media/platforms/agnostic/AOMDecoder.cpp:46 (Diff revision 3) > + else if (aInfo.mDisplay.width >= 1024) { > + decode_threads = 4; > + } > + decode_threads = std::min(decode_threads, PR_GetNumberOfProcessors()); > + > + aom_codec_dec_cfg_t config; maybe using config() or PodZero to ensure the structure is fully initialised? ::: dom/media/platforms/agnostic/AOMDecoder.cpp:55 (Diff revision 3) > + aom_codec_flags_t flags = 0; > + > + auto res = aom_codec_dec_init(aCtx, dx, &config, flags); > + if (res != AOM_CODEC_OK) { > + LOG_RESULT(res, "Codec initialization failed!"); > + return NS_ERROR_FAILURE; with MediaResult additional information can be returned to fully describe where it failed and why... such as returning the AOM error code. ::: dom/media/platforms/agnostic/AOMDecoder.cpp:65 (Diff revision 3) > +AOMDecoder::AOMDecoder(const CreateDecoderParams& aParams) > + : mImageContainer(aParams.mImageContainer) > + , mTaskQueue(aParams.mTaskQueue) > + , mInfo(aParams.VideoConfig()) > +{ > + MOZ_COUNT_CTOR(AOMDecoder); This is rather unnecessary as the MediaDataDecoder is already ref counted and those are automally moz_count ::: dom/media/webm/WebMDemuxer.cpp:30 (Diff revision 3) > #include "mozilla/IntegerPrintfMacros.h" > #include "mozilla/SizePrintfMacros.h" > #include "mozilla/Sprintf.h" > > #include <algorithm> > +#include <stdint.h> this seems out of scope for this patch....
Attachment #8859732 -
Flags: review?(jyavenard) → review+
Comment 56•7 years ago
|
||
mozreview-review |
Comment on attachment 8859733 [details] Bug 1314147 - Add AOMDecoder to AgnosticDecoderModule. https://reviewboard.mozilla.org/r/131730/#review139148
Attachment #8859733 -
Flags: review?(jyavenard) → review+
Comment 57•7 years ago
|
||
mozreview-review |
Comment on attachment 8860178 [details] Bug 1314147 - Recognize AV1 in WebMDemuxer. https://reviewboard.mozilla.org/r/132202/#review139142 ::: dom/media/webm/WebMDemuxer.cpp:336 (Diff revision 2) > break; > case NESTEGG_CODEC_VP9: > mInfo.mVideo.mMimeType = "video/webm; codecs=vp9"; > break; > + case NESTEGG_CODEC_AV1: > + mInfo.mVideo.mMimeType = "video/webm; codecs=av1"; I really don't like those mimetype... both vp8, vp9 have an officiel mime type: video/x-vnd.on2.vp8 and video/x-vnd.on2.vp9 does av1 have its own?
Attachment #8860178 -
Flags: review?(jyavenard) → review+
Comment 58•7 years ago
|
||
mozreview-review |
Comment on attachment 8847833 [details] Bug 1314147 - Add 'mach vendor aom' for maintaining av1 codec support. https://reviewboard.mozilla.org/r/120750/#review139212 This looks fine, some things to discuss before this is landed below. ::: python/mozbuild/mozbuild/mach_commands.py:1841 (Diff revision 7) > + @CommandArgument('-r', '--revision', > + help='Repository tag or commit to update to.') The update scripts I'm familiar with around the tree usually document the version that was last downloaded in the source; I think this practice is a little more robust than recording the version used in a commit message. I suppose having to change the source code means having to always pass `--ignore-modified` to this command (or change the defaults, e.g. ignoring the vendoring module always), which may not be desirable. ::: python/mozbuild/mozbuild/vendor_aom.py:34 (Diff revision 7) > + tar = tarfile.open(filename) > + self.log(logging.INFO, 'rm_vendor_dir', {}, 'rm -rf %s' % target) > + mozfile.remove(target) > + self.log(logging.INFO, 'unpack', {}, 'Unpacking upstream files.') > + tar.extractall(target) Should we add some checking here to make sure the archive unpacks in roughly the way that we expect? I guess given the source (the tarballs are generated from the git repo itself?), maybe we don't have to worry about that... ::: python/mozbuild/mozbuild/vendor_aom.py:70 (Diff revision 7) > + if not revision: > + revision = 'master' Re: versioning, I don't think we want to default to downloading `master`, because that makes it too easy to lose track of what version we last downloaded.
Attachment #8847833 -
Flags: review?(nfroyd) → review+
Comment 59•7 years ago
|
||
mozreview-review |
Comment on attachment 8857713 [details] Bug 1314147 - Port the libvpx mozbuild generator to aom. https://reviewboard.mozilla.org/r/129644/#review139218 This is kind of an rs=me, since any bugs probably mean things don't build, but a couple comments below. ::: media/libaom/generate_sources_mozbuild.sh:199 (Diff revision 6) > +x86_platforms="--enable-postproc --as=yasm" > +arm_platforms="--enable-runtime-cpu-detect --enable-realtime-only" > +gen_config_files linux/x64 "--target=x86_64-linux-gcc ${all_platforms} ${x86_platforms}" > +gen_config_files linux/ia32 "--target=x86-linux-gcc ${all_platforms} ${x86_platforms}" > +gen_config_files mac/x64 "--target=x86_64-darwin9-gcc ${all_platforms} ${x86_platforms}" > +gen_config_files mac/ia32 "--target=x86-darwin9-gcc ${all_platforms} ${x86_platforms}" IIRC, we don't support 32-bit Mac anymore, so we don't need this bit? Or does something get unhappy if it doesn't exist? ::: media/libaom/moz.build:91 (Diff revision 6) > + if f.endswith('.c'): > + if 'sse2.c' in f: > + SOURCES[f].flags += CONFIG['SSE2_FLAGS'] Instead of checking for `.c` suffixes first, maybe this should just be: ``` if f.endswith('sse2.c'): SOURCES[f].flags += ... elif ... ``` or even: ``` flagmap = [('sse2.c', CONFIG['SSE2_FLAGS']), ...] for suffix, flags in flagmap: if f.endswith(suffix): SOURCES[f].flags += flags break ``` I guess the initial filtering is more efficient, but I doubt that it matters too much.
Attachment #8857713 -
Flags: review?(nfroyd) → review+
Comment 60•7 years ago
|
||
mozreview-review |
Comment on attachment 8847834 [details] Bug 1314147 - Generate build files in mach vendor aom. https://reviewboard.mozilla.org/r/120752/#review139220
Attachment #8847834 -
Flags: review?(nfroyd) → review+
Comment 61•7 years ago
|
||
mozreview-review |
Comment on attachment 8859389 [details] Bug 1314147 - Import aom library. https://reviewboard.mozilla.org/r/131412/#review139224 rs=me
Attachment #8859389 -
Flags: review?(nfroyd) → review+
Comment 62•7 years ago
|
||
mozreview-review |
Comment on attachment 8858351 [details] Bug 1314147 - Generate build description for libaom. https://reviewboard.mozilla.org/r/130302/#review139226 It is a little unexpected (although nice) to see this split out as a separate commit, but this is not going to be the case for subsequent imports unless the importer is very careful, right? Do we care much about that?
Attachment #8858351 -
Flags: review?(nfroyd) → review+
Comment 63•7 years ago
|
||
mozreview-review |
Comment on attachment 8857712 [details] Bug 1314147 - Add --enable-av1 configure switch. https://reviewboard.mozilla.org/r/129642/#review139228
Attachment #8857712 -
Flags: review?(nfroyd) → review+
Assignee | ||
Comment 64•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8858351 [details] Bug 1314147 - Generate build description for libaom. https://reviewboard.mozilla.org/r/130302/#review139226 I don't care much about it. I split it out to make initial review easier, but in general updates I planned to do them together. It's not too hard to split them manually, but the usual problem doing updates like this is missing some filesystem change, so I think it's better for the script the import all the changes at once.
Assignee | ||
Comment 65•7 years ago
|
||
mozreview-review |
Comment on attachment 8847833 [details] Bug 1314147 - Add 'mach vendor aom' for maintaining av1 codec support. https://reviewboard.mozilla.org/r/120750/#review139264 ::: python/mozbuild/mozbuild/mach_commands.py:1841 (Diff revision 7) > + @CommandArgument('-r', '--revision', > + help='Repository tag or commit to update to.') Yes, I wasn't sure where to put the README_MOZILLA with the source split, but I think putting it in media/libaom with the commit id and a pointer to third_party/aom will work fine. I think having to pass `--ignore-modified` when working on the vendoring module is acceptable. One can always make a trial commit of local changes since they should be submitted in a separate patch from the vendor update anyway. ::: python/mozbuild/mozbuild/vendor_aom.py:34 (Diff revision 7) > + tar = tarfile.open(filename) > + self.log(logging.INFO, 'rm_vendor_dir', {}, 'rm -rf %s' % target) > + mozfile.remove(target) > + self.log(logging.INFO, 'unpack', {}, 'Unpacking upstream files.') > + tar.extractall(target) I guess this could be something like: ```python if any(filter(lambda name: name.startswith('/') or '..' in name, tar.getnames())): raise Exception('Tar archive has non-local paths') ``` The check for '..' is a little dirty, since really we should check for paths outside the toplevel. I'm not worried because the tarfile is just a faster way of getting the contents of a particular git repo, but I don't mind adding this. ::: python/mozbuild/mozbuild/vendor_aom.py:70 (Diff revision 7) > + if not revision: > + revision = 'master' Until upstream starts making releases, I think we'll usually want to update to master. Is this an ok default if we record the previous checkout in media/libaom/README_MOZILLA?
Assignee | ||
Comment 66•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8857713 [details] Bug 1314147 - Port the libvpx mozbuild generator to aom. https://reviewboard.mozilla.org/r/129644/#review139218 > IIRC, we don't support 32-bit Mac anymore, so we don't need this bit? Or does something get unhappy if it doesn't exist? Good point, thanks. > Instead of checking for `.c` suffixes first, maybe this should just be: > > ``` > if f.endswith('sse2.c'): > SOURCES[f].flags += ... > elif ... > ``` > > or even: > > ``` > flagmap = [('sse2.c', CONFIG['SSE2_FLAGS']), ...] > for suffix, flags in flagmap: > if f.endswith(suffix): > SOURCES[f].flags += flags > break > ``` > > I guess the initial filtering is more efficient, but I doubt that it matters too much. Ok.
Assignee | ||
Comment 67•7 years ago
|
||
mozreview-review |
Comment on attachment 8859732 [details] Bug 1314147 - Add AOMDecoder. https://reviewboard.mozilla.org/r/131728/#review139274 ::: dom/media/webm/WebMDemuxer.cpp:30 (Diff revision 3) > #include "mozilla/IntegerPrintfMacros.h" > #include "mozilla/SizePrintfMacros.h" > #include "mozilla/Sprintf.h" > > #include <algorithm> > +#include <stdint.h> In particular, we don't need two of them! I think this is a rebase mistake, but I've opened bug 1362107 to shut my editor up about missing includes.
Assignee | ||
Comment 68•7 years ago
|
||
mozreview-review |
Comment on attachment 8860178 [details] Bug 1314147 - Recognize AV1 in WebMDemuxer. https://reviewboard.mozilla.org/r/132202/#review139294 ::: dom/media/webm/WebMDemuxer.cpp:336 (Diff revision 2) > break; > case NESTEGG_CODEC_VP9: > mInfo.mVideo.mMimeType = "video/webm; codecs=vp9"; > break; > + case NESTEGG_CODEC_AV1: > + mInfo.mVideo.mMimeType = "video/webm; codecs=av1"; Well, I'm unclear whether we want to signal av1-from-webm here, or just raw av1. RFC 7741 registered video/VP8. That's what webrtc uses. There's a wg draft for video/VP9; probably waiting on the bitstream format RFC, also in draft, before publication as an RFC. I don't think the x-vnd.on2 types are more official. AV1 doesn't have a mimetype yet. I expect the IETF netvc working group will produce a registration draft eventually, since the charter says HTML and WebRTC are intended uses. I expect it will follow what vp8 and vp9 did. In any case this is no worse than what we have currently, so if you want to improve it, lets to that in a separate bug.
Comment 69•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8860178 [details] Bug 1314147 - Recognize AV1 in WebMDemuxer. https://reviewboard.mozilla.org/r/132202/#review139294 > Well, I'm unclear whether we want to signal av1-from-webm here, or just raw av1. > > RFC 7741 registered video/VP8. That's what webrtc uses. There's a wg draft for video/VP9; probably waiting on the bitstream format RFC, also in draft, before publication as an RFC. I don't think the x-vnd.on2 types are more official. > > AV1 doesn't have a mimetype yet. I expect the IETF netvc working group will produce a registration draft eventually, since the charter says HTML and WebRTC are intended uses. I expect it will follow what vp8 and vp9 did. > > In any case this is no worse than what we have currently, so if you want to improve it, lets to that in a separate bug. the mimetype codec reported by the demuxer are to be the raw codec mimetype. It just happened that from the start the WebM demuxer used a complex container one... But that's ugly. That forces us to convert them back and forth in some places like here: https://dxr.mozilla.org/mozilla-central/source/dom/media/platforms/android/AndroidDecoderModule.cpp#40 So I think it would be nice, as an extra task linked to this change, to use the proper codec mimetype, and not the container ones. in the mean time, for av1 we could use video/av1 or video/video1...
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8847834 -
Attachment is obsolete: true
Attachment #8847834 -
Flags: review?(kinetik)
Assignee | ||
Comment 79•7 years ago
|
||
mozreview-review |
Comment on attachment 8847833 [details] Bug 1314147 - Add 'mach vendor aom' for maintaining av1 codec support. https://reviewboard.mozilla.org/r/120750/#review139404 Would be good if you could look at this again, Nathan.
Assignee | ||
Comment 80•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8857713 [details] Bug 1314147 - Port the libvpx mozbuild generator to aom. https://reviewboard.mozilla.org/r/129644/#review139218 > Good point, thanks. I've removed the mac/ia32 config.
Comment 81•7 years ago
|
||
mozreview-review |
Comment on attachment 8857713 [details] Bug 1314147 - Port the libvpx mozbuild generator to aom. https://reviewboard.mozilla.org/r/129644/#review139474
Attachment #8857713 -
Flags: review?(kinetik) → review+
Comment 82•7 years ago
|
||
mozreview-review |
Comment on attachment 8858351 [details] Bug 1314147 - Generate build description for libaom. https://reviewboard.mozilla.org/r/130302/#review139480
Attachment #8858351 -
Flags: review?(kinetik) → review+
Comment 83•7 years ago
|
||
mozreview-review |
Comment on attachment 8859389 [details] Bug 1314147 - Import aom library. https://reviewboard.mozilla.org/r/131412/#review139484
Attachment #8859389 -
Flags: review?(kinetik) → review+
Comment 84•7 years ago
|
||
(In reply to Ralph Giles (:rillian) | needinfo me from comment #79) > Comment on attachment 8847833 [details] > Bug 1314147 - Add 'mach vendor aom' for maintaining av1 codec support. > > https://reviewboard.mozilla.org/r/120750/#review139404 > > Would be good if you could look at this again, Nathan. WFM, thank you!
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Updated•7 years ago
|
Attachment #8860180 -
Attachment is obsolete: true
Assignee | ||
Comment 93•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8859732 [details] Bug 1314147 - Add AOMDecoder. https://reviewboard.mozilla.org/r/131728/#review139144 > maybe using config() or PodZero to ensure the structure is fully initialised? Ok. We were initializing all the fields, but you're correct that PodZero is safer against future changes. > with MediaResult additional information can be returned to fully describe where it failed and why... such as returning the AOM error code. Ok. > This is rather unnecessary as the MediaDataDecoder is already ref counted and those are automally moz_count I've removed the counters.
Assignee | ||
Comment 94•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8857713 [details] Bug 1314147 - Port the libvpx mozbuild generator to aom. https://reviewboard.mozilla.org/r/129644/#review134722 > `VPX_USE_YASM` isn't defined for `--with-system-libvpx`. Maybe replace with `HAVE_YASM` + `INTEL_ARCHITECTURE` or adjust old-configure.in while keeping in mind `--with-system-aom` maybe added in future. > > ``` > /usr/bin/clang -std=gnu99 -o aom_convolve_copy_sse2.o -fPIC -Wa,--noexecstack -include objdir/mozilla-config.h -DMOZILLA_CLIENT -Imedia/libaom/config/linux/x64/ -I. -Ithird_party/aom -c third_party/aom/aom_dsp/x86/aom_convolve_copy_sse2.asm > clang: warning: argument unused during compilation: '-std=gnu99' [-Wunused-command-line-argument] > clang: warning: argument unused during compilation: '-include objdir/mozilla-config.h' [-Wunused-command-line-argument] > clang: warning: argument unused during compilation: '-D MOZILLA_CLIENT' [-Wunused-command-line-argument] > third_party/aom/aom_dsp/x86/aom_convolve_copy_sse2.asm:2:17: error: unexpected token in argument list > ; Copyright (c) 2016, Alliance for Open Media. All rights reserved > ^ > third_party/aom/aom_dsp/x86/aom_convolve_copy_sse2.asm:4:15: error: unexpected token in argument list > ; This source code is subject to the terms of the BSD 2 Clause License and > ^ > [...] > ``` Since --enable-av1 isn't on by default, I want to delay fixing this until after this patch set lands. I've removed the VPX_USE_YASM dependency here and filed bug 1362465 for the follow-up with the other two VPX asm config keys. Thanks again for looking after these issues.
Comment 95•7 years ago
|
||
Pushed by rgiles@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3398e9327710 Port the libvpx mozbuild generator to aom. r=froydnj,kinetik https://hg.mozilla.org/integration/autoland/rev/ea693af1b5fb Add 'mach vendor aom' for maintaining av1 codec support. r=froydnj https://hg.mozilla.org/integration/autoland/rev/e9cfbd2b4fe2 Import aom library. r=froydnj,kinetik https://hg.mozilla.org/integration/autoland/rev/69beada73635 Generate build description for libaom. r=froydnj,kinetik https://hg.mozilla.org/integration/autoland/rev/c57f94d10914 Add --enable-av1 configure switch. r=froydnj https://hg.mozilla.org/integration/autoland/rev/95e800f66f2b Add AOMDecoder. r=jya https://hg.mozilla.org/integration/autoland/rev/d5f91c51a3ab Add AOMDecoder to AgnosticDecoderModule. r=jya https://hg.mozilla.org/integration/autoland/rev/5c767e2e3ded Recognize AV1 in WebMDemuxer. r=jya
Comment 96•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3398e9327710 https://hg.mozilla.org/mozilla-central/rev/ea693af1b5fb https://hg.mozilla.org/mozilla-central/rev/e9cfbd2b4fe2 https://hg.mozilla.org/mozilla-central/rev/69beada73635 https://hg.mozilla.org/mozilla-central/rev/c57f94d10914 https://hg.mozilla.org/mozilla-central/rev/95e800f66f2b https://hg.mozilla.org/mozilla-central/rev/d5f91c51a3ab https://hg.mozilla.org/mozilla-central/rev/5c767e2e3ded
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment 97•7 years ago
|
||
Ralph, should this get any manual testing? If yes, can you provide some details?
Flags: needinfo?(giles)
Assignee | ||
Comment 98•7 years ago
|
||
No manual testing is necessary at the moment. The codec isn't enabled by default yet, and the format is still evolving. Thanks for checking!
Flags: needinfo?(giles)
Comment 99•7 years ago
|
||
Just for information - av1 was enabled for Linux (and MAC) in https://bugzilla.mozilla.org/show_bug.cgi?id=1370978 but https://people-mozilla.org/~rgiles/2017/rush_hour_444.av1.webm doesnt play in Linux Nightly 55.0a1 (2017-06-11) (64-bit) Browser console says Media resource https://people-mozilla.org/~rgiles/2017/rush_hour_444.av1.webm could not be decoded. Media resource https://people-mozilla.org/~rgiles/2017/rush_hour_444.av1.webm could not be decoded, error: Error Code: NS_ERROR_DOM_MEDIA_DECODE_ERR (0x806e0004) Details: RefPtr<mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true> > mozilla::AOMDecoder::ProcessDecode(mozilla::MediaRawData*): AOM error decoding AV1 sample: Corrupt frame detected same result with https://people-mozilla.org/~rgiles/2017/sintel_trailer_2k_480p24.av1.webm
Assignee | ||
Comment 100•7 years ago
|
||
Oops, I bumped the decoder version in Firefox without updating the test files. I've bumped https://people-mozilla.org/~rgiles/2017/rush_hour_444.av1.webm and it plays for me now. I'll upload an updated encode of the sintel_trailer when the encode job finishes. Thanks for testing!
Updated•6 years ago
|
Alias: AV1
You need to log in
before you can comment on or make changes to this bug.
Description
•