Re: [FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd

2015-02-24 Thread Michael Niedermayer
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 michae...@gmx.at 
  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

2015-02-20 Thread Michael Niedermayer
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


Re: [FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd

2015-02-20 Thread Kevin Wheatley
On Fri, Feb 20, 2015 at 11:36 AM, Michael Niedermayer michae...@gmx.at 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

2015-02-20 Thread Michael Niedermayer
On Fri, Feb 20, 2015 at 04:04:52PM +, Kevin Wheatley wrote:
 On Fri, Feb 20, 2015 at 11:36 AM, Michael Niedermayer michae...@gmx.at 
 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


[FFmpeg-devel] [PATCH] avformat: Outputting DNxHD into .mov containers 'corrupts' following atoms until end of stsd

2015-02-16 Thread Kevin Wheatley
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 kevin.j.wheat...@gmail.com
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 kevin.j.wheat...@gmail.com
---
 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 next:
 - 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