diff mbox

[FFmpeg-devel] movenc: write clap tag

Message ID 5E7D2AAF-480A-4AF5-AC59-1D13A8086827@dericed.com
State New
Headers show

Commit Message

Dave Rice Nov. 16, 2017, 5:14 p.m. UTC
> On Nov 16, 2017, at 11:30 AM, Dave Rice <dave@dericed.com> wrote:
> 
>> On Jul 9, 2017, at 7:26 PM, Dave Rice <dave@dericed.com> wrote:
>> 
>>> On Jul 7, 2017, at 7:06 PM, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
>>> 
>>> On 7/7/2017 10:13 PM, James Almer wrote:
>>>> Isn't this necessary only for files with raw video? As is, this box
>>>> would be written on any mov file with a video stream.
>>> 
>>> This was addressed a previous email:
>>> 
>>>  http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-July/213350.html
>>> 
>>> I guess the spec is up for interpretation.
>> 
>> The quicktime spec says "This is a mandatory extension for all uncompressed Y´CbCr data formats”. It doesn’t clarify if the clap atom is recommended or not in mov files that are not “uncompressed Y´CbCr”, though it would make sense if the video container needs to store cropping data. I think constraining the change for only  “uncompressed Y´CbCr” would be more cautious though. I’ll revise my patch to include the condition and resubmit.
>> 
>> If the patch only impacts “uncompressed Y´CbCr” would any fate updates be needed?
>> Dave Rice
> 
> Here’s an update to only write the clap atom for the specific uncompressed encodings listed in TN2162.
> 
> From 37457c1ee135f39452b91b047af4adf1ec43464b Mon Sep 17 00:00:00 2001
> From: Dave Rice <dave@dericed.com>
> Date: Thu, 16 Nov 2017 11:29:06 -0500
> Subject: [PATCH] avformat/movenc: write clap atom for uncompressed yuv in mov

Sorry, this patch should supersede the prior email's patch. I realized that Apple requires new uncompressed ycbcr files to use version 2 in the Image Description, so I reused the uncompressed_ycbcr variable to add that in as well.

From 3ea99e7d22f67b8a556152acbcbc8bf2eeec8a39 Mon Sep 17 00:00:00 2001
From: Dave Rice <dave@dericed.com>
Date: Thu, 16 Nov 2017 11:29:06 -0500
Subject: [PATCH 1/2] avformat/movenc: write clap atom for uncompressed yuv in
 mov

fixes 6145
---
 libavformat/movenc.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer Nov. 17, 2017, 5:20 p.m. UTC | #1
On Thu, Nov 16, 2017 at 12:14:30PM -0500, Dave Rice wrote:
> 
> > On Nov 16, 2017, at 11:30 AM, Dave Rice <dave@dericed.com> wrote:
> > 
> >> On Jul 9, 2017, at 7:26 PM, Dave Rice <dave@dericed.com> wrote:
> >> 
> >>> On Jul 7, 2017, at 7:06 PM, Derek Buitenhuis <derek.buitenhuis@gmail.com> wrote:
> >>> 
> >>> On 7/7/2017 10:13 PM, James Almer wrote:
> >>>> Isn't this necessary only for files with raw video? As is, this box
> >>>> would be written on any mov file with a video stream.
> >>> 
> >>> This was addressed a previous email:
> >>> 
> >>>  http://lists.ffmpeg.org/pipermail/ffmpeg-devel/2017-July/213350.html
> >>> 
> >>> I guess the spec is up for interpretation.
> >> 
> >> The quicktime spec says "This is a mandatory extension for all uncompressed Y´CbCr data formats”. It doesn’t clarify if the clap atom is recommended or not in mov files that are not “uncompressed Y´CbCr”, though it would make sense if the video container needs to store cropping data. I think constraining the change for only  “uncompressed Y´CbCr” would be more cautious though. I’ll revise my patch to include the condition and resubmit.
> >> 
> >> If the patch only impacts “uncompressed Y´CbCr” would any fate updates be needed?
> >> Dave Rice
> > 
> > Here’s an update to only write the clap atom for the specific uncompressed encodings listed in TN2162.
> > 
> > From 37457c1ee135f39452b91b047af4adf1ec43464b Mon Sep 17 00:00:00 2001
> > From: Dave Rice <dave@dericed.com>
> > Date: Thu, 16 Nov 2017 11:29:06 -0500
> > Subject: [PATCH] avformat/movenc: write clap atom for uncompressed yuv in mov
> 
> Sorry, this patch should supersede the prior email's patch. I realized that Apple requires new uncompressed ycbcr files to use version 2 in the Image Description, so I reused the uncompressed_ycbcr variable to add that in as well.
> 
> From 3ea99e7d22f67b8a556152acbcbc8bf2eeec8a39 Mon Sep 17 00:00:00 2001
> From: Dave Rice <dave@dericed.com>
> Date: Thu, 16 Nov 2017 11:29:06 -0500
> Subject: [PATCH 1/2] avformat/movenc: write clap atom for uncompressed yuv in
>  mov
> 
> fixes 6145
> ---
>  libavformat/movenc.c | 32 +++++++++++++++++++++++++++++++-
>  1 file changed, 31 insertions(+), 1 deletion(-)

please add a fate test

and if this is tested with some official player and works then LGTM

thx

[...]
diff mbox

Patch

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index cc3fc19d9b..98fcc7a44b 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1686,6 +1686,21 @@  static int mov_write_sv3d_tag(AVFormatContext *s, AVIOContext *pb, AVSphericalMa
     return update_size(pb, sv3d_pos);
 }
 
+static int mov_write_clap_tag(AVIOContext *pb, MOVTrack *track)
+{
+    avio_wb32(pb, 40);
+    ffio_wfourcc(pb, "clap");
+    avio_wb32(pb, track->par->width); /* apertureWidth_N */
+    avio_wb32(pb, 1); /* apertureWidth_D (= 1) */
+    avio_wb32(pb, track->height); /* apertureHeight_N */
+    avio_wb32(pb, 1); /* apertureHeight_D (= 1) */
+    avio_wb32(pb, 0); /* horizOff_N (= 0) */
+    avio_wb32(pb, 1); /* horizOff_D (= 1) */
+    avio_wb32(pb, 0); /* vertOff_N (= 0) */
+    avio_wb32(pb, 1); /* vertOff_D (= 1) */
+    return 40;
+}
+
 static int mov_write_pasp_tag(AVIOContext *pb, MOVTrack *track)
 {
     AVRational sar;
@@ -1832,6 +1847,13 @@  static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr
     char compressor_name[32] = { 0 };
     int avid = 0;
 
+    int uncompressed_ycbcr = ((track->par->codec_id == AV_CODEC_ID_RAWVIDEO && track->par->format == AV_PIX_FMT_UYVY422)
+                           || (track->par->codec_id == AV_CODEC_ID_RAWVIDEO && track->par->format == AV_PIX_FMT_YUYV422)
+                           ||  track->par->codec_id == AV_CODEC_ID_V308
+                           ||  track->par->codec_id == AV_CODEC_ID_V408
+                           ||  track->par->codec_id == AV_CODEC_ID_V410
+                           ||  track->par->codec_id == AV_CODEC_ID_V210);
+
     avio_wb32(pb, 0); /* size */
     if (mov->encryption_scheme != MOV_ENC_NONE) {
         ffio_wfourcc(pb, "encv");
@@ -1842,7 +1864,11 @@  static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr
     avio_wb16(pb, 0); /* Reserved */
     avio_wb16(pb, 1); /* Data-reference index */
 
-    avio_wb16(pb, 0); /* Codec stream version */
+    if (uncompressed_ycbcr) {
+        avio_wb16(pb, 2); /* Codec stream version */
+    } else {
+        avio_wb16(pb, 0); /* Codec stream version */
+    }
     avio_wb16(pb, 0); /* Codec stream revision (=0) */
     if (track->mode == MODE_MOV) {
         ffio_wfourcc(pb, "FFMP"); /* Vendor */
@@ -1969,6 +1995,10 @@  static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr
     if (track->par->sample_aspect_ratio.den && track->par->sample_aspect_ratio.num) {
         mov_write_pasp_tag(pb, track);
     }
+
+    if (uncompressed_ycbcr){
+        mov_write_clap_tag(pb, track);
+    }
 
     if (mov->encryption_scheme != MOV_ENC_NONE) {
         ff_mov_cenc_write_sinf_tag(track, pb, mov->encryption_kid);