Re: [FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd
On Fri, Feb 20, 2015 at 07:00:48PM +0100, Michael Niedermayer wrote: > On Fri, Feb 20, 2015 at 04:04:52PM +, Kevin Wheatley wrote: > > On Fri, Feb 20, 2015 at 11:36 AM, Michael Niedermayer > > wrote: > > > applied the case for DNxHD, for the more general case, please > > > explain which case(s) and software need them, and how to reproduce > > > that > > > > My experience and by the looks of things other people using > > libquicktime have seen issues with Final Cut having problems reading > > the files if the stds > > > > http://libquicktime.cvs.sourceforge.net/viewvc/libquicktime/libquicktime/src/stsdtable.c?view=markup > > > > quicktime_write_stsd_video() line 643 is where they sometimes pad. > > > > http://comments.gmane.org/gmane.comp.video.libquicktime.devel/1348 > > > > appears to be the discussion around why they do it > > hmm, ok, i guess doing the same "sometimes" padding is what we should > do then too > if you want to submit a patch along these lines, ill apply it unless > it breaks something posted a patch that would do this [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The real ebay dictionary, page 2 "100% positive feedback" - "All either got their money back or didnt complain" "Best seller ever, very honest" - "Seller refunded buyer after failed scam" signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd
On Fri, Feb 20, 2015 at 04:04:52PM +, Kevin Wheatley wrote: > On Fri, Feb 20, 2015 at 11:36 AM, Michael Niedermayer > wrote: > > applied the case for DNxHD, for the more general case, please > > explain which case(s) and software need them, and how to reproduce > > that > > My experience and by the looks of things other people using > libquicktime have seen issues with Final Cut having problems reading > the files if the stds > > http://libquicktime.cvs.sourceforge.net/viewvc/libquicktime/libquicktime/src/stsdtable.c?view=markup > > quicktime_write_stsd_video() line 643 is where they sometimes pad. > > http://comments.gmane.org/gmane.comp.video.libquicktime.devel/1348 > > appears to be the discussion around why they do it hmm, ok, i guess doing the same "sometimes" padding is what we should do then too if you want to submit a patch along these lines, ill apply it unless it breaks something [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Asymptotically faster algorithms should always be preferred if you have asymptotical amounts of data signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd
On Fri, Feb 20, 2015 at 11:36 AM, Michael Niedermayer wrote: > applied the case for DNxHD, for the more general case, please > explain which case(s) and software need them, and how to reproduce > that My experience and by the looks of things other people using libquicktime have seen issues with Final Cut having problems reading the files if the stds http://libquicktime.cvs.sourceforge.net/viewvc/libquicktime/libquicktime/src/stsdtable.c?view=markup quicktime_write_stsd_video() line 643 is where they sometimes pad. http://comments.gmane.org/gmane.comp.video.libquicktime.devel/1348 appears to be the discussion around why they do it > I dont see where the text would allow one to add such padding by > ones own choice. I just see a note that says that parsers need to > cope with it. Thats not the same as saying its ok to add it in all > cases. > But maybe iam missing something, i didnt read the whole document no I would agree it only indicates it is optional Kevin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd
On Mon, Feb 16, 2015 at 10:49:52AM +, Kevin Wheatley wrote: > ffmpeg and qtdump could not decode pasp/colr atoms in the files made by > ffmpeg, > when outputting DNxHD due to the incorrect padding placement. Now we add the > padding in the correct place, we also always add padding for Final Cut > compatibility. > > Tidy up FATE changes due to padding changes. applied the case for DNxHD, for the more general case, please explain which case(s) and software need them, and how to reproduce that I dont see where the text would allow one to add such padding by ones own choice. I just see a note that says that parsers need to cope with it. Thats not the same as saying its ok to add it in all cases. But maybe iam missing something, i didnt read the whole document [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In fact, the RIAA has been known to suggest that students drop out of college or go to community college in order to be able to afford settlements. -- The RIAA signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd
ffmpeg and qtdump could not decode pasp/colr atoms in the files made by ffmpeg, when outputting DNxHD due to the incorrect padding placement. Now we add the padding in the correct place, we also always add padding for Final Cut compatibility. Tidy up FATE changes due to padding changes. Kevin From b67ca5347a26227621054c82a688cc0ca44c11a1 Mon Sep 17 00:00:00 2001 From: Kevin Wheatley Date: Mon, 16 Feb 2015 10:40:36 + Subject: [PATCH] Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd ffmpeg and qtdump could not decode pasp/colr atoms in the files made by ffmpeg, when outputting DNxHD due to the incorrect padding placement. Now we add the padding in the correct place, we also always add padding for Final Cut compatibility. Tidy up FATE changes due to padding changes. Signed-off-by: Kevin Wheatley --- Changelog |3 ++ libavformat/movenc.c |7 +++- tests/ref/lavf/ismv | 12 +++--- tests/ref/lavf/mov| 16 tests/ref/seek/lavf-mov | 44 tests/ref/vsynth/vsynth1-avui |4 +- tests/ref/vsynth/vsynth1-dnxhd-1080i-colr |2 +- tests/ref/vsynth/vsynth1-mpeg4|4 +- tests/ref/vsynth/vsynth1-prores |4 +- tests/ref/vsynth/vsynth1-prores_ks|4 +- tests/ref/vsynth/vsynth1-qtrle|4 +- tests/ref/vsynth/vsynth1-qtrlegray|4 +- tests/ref/vsynth/vsynth1-svq1 |4 +- tests/ref/vsynth/vsynth2-avui |4 +- tests/ref/vsynth/vsynth2-dnxhd-1080i-colr |2 +- tests/ref/vsynth/vsynth2-mpeg4|4 +- tests/ref/vsynth/vsynth2-prores |4 +- tests/ref/vsynth/vsynth2-prores_ks|4 +- tests/ref/vsynth/vsynth2-qtrle|4 +- tests/ref/vsynth/vsynth2-qtrlegray|4 +- tests/ref/vsynth/vsynth2-svq1 |4 +- tests/ref/vsynth/vsynth3-dnxhd-1080i-colr |2 +- tests/ref/vsynth/vsynth3-mpeg4|4 +- tests/ref/vsynth/vsynth3-prores |4 +- tests/ref/vsynth/vsynth3-prores_ks|4 +- tests/ref/vsynth/vsynth3-qtrle|4 +- tests/ref/vsynth/vsynth3-svq1 |4 +- tests/ref/vsynth/vsynth_lena-avui |4 +- tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr |2 +- tests/ref/vsynth/vsynth_lena-mpeg4|4 +- tests/ref/vsynth/vsynth_lena-prores |4 +- tests/ref/vsynth/vsynth_lena-prores_ks|4 +- tests/ref/vsynth/vsynth_lena-qtrle|4 +- tests/ref/vsynth/vsynth_lena-qtrlegray|4 +- tests/ref/vsynth/vsynth_lena-svq1 |4 +- 35 files changed, 100 insertions(+), 94 deletions(-) diff --git a/Changelog b/Changelog index 0bf7018..8080286 100644 --- a/Changelog +++ b/Changelog @@ -30,6 +30,9 @@ version : - DV RTP payload format (RFC 6469) depacketizer - DXVA2-accelerated HEVC decoding - AAC ELD 480 decoding +- Fix stsd atom corruption in DNxHD QuickTimes +- Add 4 bytes stsd zero padding to video track for all QuickTime outputs + version 2.5: - HEVC/H.265 RTP payload format (draft v6) packetizer diff --git a/libavformat/movenc.c b/libavformat/movenc.c index f95d066..e8b3a6b 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1077,8 +1077,6 @@ static int mov_write_avid_tag(AVIOContext *pb, MOVTrack *track) for (i = 0; i < 10; i++) avio_wb64(pb, 0); -/* extra padding for stsd needed */ -avio_wb32(pb, 0); return 0; } @@ -1674,6 +1672,11 @@ static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr mov_write_pasp_tag(pb, track); } +/* extra padding for stsd needed */ +/* https://developer.apple.com/library/mac/documentation/QuickTime/QTFF/QTFFChap2/qtff2.html#//apple_ref/doc/uid/TP4939-CH204-61112 */ +/* suggests it is optional, but there are suggestions that Final Cut may need it. */ +avio_wb32(pb, 0); + return update_size(pb, pos); } diff --git a/tests/ref/lavf/ismv b/tests/ref/lavf/ismv index f29b5ff..ea8ea3a 100644 --- a/tests/ref/lavf/ismv +++ b/tests/ref/lavf/ismv @@ -1,9 +1,9 @@ -a9ccbb4cd1436d222ef4425567b4e03d *./tests/data/lavf/lavf.ismv -312542 ./tests/data/lavf/lavf.ismv +e26cf1bc88f31010c4c34af8eb9580ec *./tests/data/lavf/lavf.ismv +312546 ./tests/data/lavf/lavf.ismv ./tests/data/lavf/lavf.ismv CRC=0x9d9a638a -440d85f9fd5b9f63c2676638782b5c15 *./tests/data/lavf/lavf.ismv -321448 ./tests/data/lavf/lavf.ismv +ed0124f2dab1204869f3afed20d631c9 *./tests/data/lavf/lavf.ismv +321452 ./tests/data/lavf/lavf.ismv ./tests/data/lavf/lavf.ismv CRC=0xe8130120 -a9ccbb4cd1436d222ef4425567b4e03d *./tests/data/lavf/lavf.ismv -312542 ./tests/data/lavf/lavf.ismv +e26cf1bc88f31010c4c34af8eb9580ec *.