[FFmpeg-devel] avformat/movenc: read track title from correct key

Submitted by Gyan on June 16, 2018, 10:49 a.m.

Details

Message ID d9ea4c75-96a7-a26d-b828-21aaa3c82f06@gmail.com
State Accepted
Commit 830695be366ff753a3f7b8dd97c10d3e6187d41c
Headers show

Commit Message

Gyan June 16, 2018, 10:49 a.m.
Activates functionality added a few years ago.

Regards,
Gyan
From bead9f22630f2b8efc4a3859568cb0fc46102dd3 Mon Sep 17 00:00:00 2001
From: Gyan Doshi <ffmpeg@gyani.pro>
Date: Sat, 16 Jun 2018 15:31:51 +0530
Subject: [PATCH] avformat/movenc: read track title from correct key

da9cc22d5bd allowed the MOV muxer to relay a custom stream handler name,
whether populated from the input stream or user-set. However, the entry
key didn't match the key set by the MOV demuxer, so it wasn't
effective. Fixed.

Due to the change, four FATE refs have to be updated. Verified that the
target payload of the tests hasn't changed in terms of CRC.
---
 libavformat/movenc.c                | 2 +-
 tests/ref/fate/binsub-movtextenc    | 2 +-
 tests/ref/fate/copy-psp             | 4 ++--
 tests/ref/fate/copy-trac236         | 4 ++--
 tests/ref/lavf-fate/mov_qtrle_mace6 | 4 ++--
 5 files changed, 8 insertions(+), 8 deletions(-)

Comments

John Stebbins June 16, 2018, 7:53 p.m.
On 06/16/2018 03:49 AM, Gyan Doshi wrote:
> Activates functionality added a few years ago.
>
> Regards,
> Gyan
>

FYI, HandBrake has been using the "handler" metadata key since 2014 when I added this code.  And it was completely self-consistent (since it was the only occurrence of usage of this key) until Hendrik Leppkes added support for reading the handler name in
2015 but used handler_name metadata key instead.  I'm not against the change.  Just need to make a note to myself to fix HandBrake when we update to a version of ffmpeg with the change.
Gyan June 17, 2018, 4:37 a.m.
On 17-06-2018 01:23 AM, John Stebbins wrote:
> On 06/16/2018 03:49 AM, Gyan Doshi wrote:
>> Activates functionality added a few years ago.
>>
>> Regards,
>> Gyan
>>
> 
> FYI, HandBrake has been using the "handler" metadata key since 2014 when I added this code.  And it was completely self-consistent (since it was the only occurrence of usage of this key) until Hendrik Leppkes added support for reading the handler name in
> 2015 but used handler_name metadata key instead.  I'm not against the change.  Just need to make a note to myself to fix HandBrake when we update to a version of ffmpeg with the change.

I can just use an if-else to check for both, giving priority to 'handler'.

Thanks,
Gyan
Gyan June 18, 2018, 2:30 p.m.
On 17-06-2018 10:07 AM, Gyan Doshi wrote:

>> Just need to make a note to myself to fix HandBrake when we 
>> update to a version of ffmpeg with the change.
> 
> I can just use an if-else to check for both, giving priority to 'handler'.

When I wrote the above, I didn't realize that Handbrake had already 
switched.

Will push in a day as-is if there are no objections.


Regards,
Gyan
John Stebbins June 18, 2018, 5:47 p.m.
On 06/18/2018 07:30 AM, Gyan Doshi wrote:
>
> On 17-06-2018 10:07 AM, Gyan Doshi wrote:
>
>>> Just need to make a note to myself to fix HandBrake when we 
>>> update to a version of ffmpeg with the change.
>> I can just use an if-else to check for both, giving priority to 'handler'.
> When I wrote the above, I didn't realize that Handbrake had already 
> switched.
>
> Will push in a day as-is if there are no objections.
>
>

No objections from me.  Was only making sure you had full information about the history of this code.
Gyan June 19, 2018, 5:38 a.m.
On 18-06-2018 11:17 PM, John Stebbins wrote:

>>
>> Will push in a day as-is if there are no objections.
>>
>>
> 
> No objections from me.  Was only making sure you had full information about the history of this code.

Pushed.

Regards,
Gyan

Patch hide | download patch | download mbox

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index db266b7765..3661d24f4f 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2599,7 +2599,7 @@  static int mov_write_hdlr_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
             // of the track. So if an alternate handler description is
             // specified, use it.
             AVDictionaryEntry *t;
-            t = av_dict_get(track->st->metadata, "handler", NULL, 0);
+            t = av_dict_get(track->st->metadata, "handler_name", NULL, 0);
             if (t && utf8len(t->value))
                 descr = t->value;
         }
diff --git a/tests/ref/fate/binsub-movtextenc b/tests/ref/fate/binsub-movtextenc
index 22ee85a2f8..dacee0931e 100644
--- a/tests/ref/fate/binsub-movtextenc
+++ b/tests/ref/fate/binsub-movtextenc
@@ -1 +1 @@ 
-af6a8f38d7c11d9af7823cc44554d2ad
+66b25412f7ca699ee525ba162246edb6
diff --git a/tests/ref/fate/copy-psp b/tests/ref/fate/copy-psp
index 81eb172549..44ec461265 100644
--- a/tests/ref/fate/copy-psp
+++ b/tests/ref/fate/copy-psp
@@ -1,5 +1,5 @@ 
-cada61453a2483ef8ba1fb82c8bbff25 *tests/data/fate/copy-psp.psp
-2041433 tests/data/fate/copy-psp.psp
+65a177552e03123c9a62ddb942970d05 *tests/data/fate/copy-psp.psp
+2041445 tests/data/fate/copy-psp.psp
 #extradata 0:       51, 0xaf6d1012
 #extradata 1:        2, 0x00b200a1
 #tb 0: 1/90000
diff --git a/tests/ref/fate/copy-trac236 b/tests/ref/fate/copy-trac236
index c5240ca3d3..6470c05a05 100644
--- a/tests/ref/fate/copy-trac236
+++ b/tests/ref/fate/copy-trac236
@@ -1,5 +1,5 @@ 
-d6e3d97b522ce881ed29c5da74cc7e63 *tests/data/fate/copy-trac236.mov
-630810 tests/data/fate/copy-trac236.mov
+8b57d14c14bb4cdaca660d161e08eb8f *tests/data/fate/copy-trac236.mov
+630861 tests/data/fate/copy-trac236.mov
 #tb 0: 100/2997
 #media_type 0: video
 #codec_id 0: rawvideo
diff --git a/tests/ref/lavf-fate/mov_qtrle_mace6 b/tests/ref/lavf-fate/mov_qtrle_mace6
index f8428aaa49..3afb909574 100644
--- a/tests/ref/lavf-fate/mov_qtrle_mace6
+++ b/tests/ref/lavf-fate/mov_qtrle_mace6
@@ -1,3 +1,3 @@ 
-dcc9c4c182a5809dee9a9366f4533797 *./tests/data/lavf-fate/lavf.mov
-1270387 ./tests/data/lavf-fate/lavf.mov
+f9715cc38a3206bcdf105786905255af *./tests/data/lavf-fate/lavf.mov
+1270415 ./tests/data/lavf-fate/lavf.mov
 ./tests/data/lavf-fate/lavf.mov CRC=0x9320cd26