[FFmpeg-devel] [PATCH] read more QT tags

[Michael Niedermayer]

On Fri, Nov 02, 2012 at 12:00:08AM -0400, Dave Rice wrote:
> 
> On Nov 1, 2012, at 10:49 PM, Michael Niedermayer wrote:
> 
> > On Thu, Nov 01, 2012 at 10:10:21PM -0400, Dave Rice wrote:
> >> 
> >> On Nov 1, 2012, at 9:57 PM, Michael Niedermayer wrote:
> >> 
> >>> On Thu, Nov 01, 2012 at 08:26:07PM -0400, Dave Rice wrote:
> >>>> Hi,
> >>>> 
> >>>> This patch enables ffmpeg to read several additional QuickTime metadata tags and corrects a few of the existing ones.
> >>>> 
> >>>> I tested this against QuickTime tags that were created by QuickTime 7 and by qtmux. I uploaded a sample QuickTime file that contains many of the tags referenced in the patch to http://dericed.com/samples/metadata.mov
> >>>> 
> >>> 
> >>>> Looking forward to feedback. Thanks.
> >>> 
> >>> feedback from gcc ;)
> >>> 
> >>> libavformat/mov.c: In function ‘mov_read_udta_string’:
> >>> libavformat/mov.c:329:5: error: duplicate case value
> >>> libavformat/mov.c:300:5: error: previously used here
> >>> libavformat/mov.c: At top level:
> >> 
> >> fixed.
> > 
> >> mov.c |   40 ++++++++++++++++++++++++++++++++++------
> >> 1 file changed, 34 insertions(+), 6 deletions(-)
> >> d4062474413b5d75fab271577f549360c95debb8  quicktime_tags.patch
> >> diff --git a/libavformat/mov.c b/libavformat/mov.c
> >> index d7d7f21..e8b0f79 100644
> >> --- a/libavformat/mov.c
> >> +++ b/libavformat/mov.c
> >> @@ -294,29 +294,57 @@ static int mov_read_udta_string(MOVContext *c, AVIOContext *pb, MOVAtom atom)
> >> 
> >>     switch (atom.type) {
> >>     case MKTAG(0xa9,'n','a','m'): key = "title";     break;
> >> -    case MKTAG(0xa9,'a','u','t'):
> >> +    case MKTAG(0xa9,'a','u','t'): key = "author";    break;
> >>     case MKTAG(0xa9,'A','R','T'): key = "artist";    break;
> >>     case MKTAG( 'a','A','R','T'): key = "album_artist";    break;
> >> -    case MKTAG(0xa9,'w','r','t'): key = "composer";  break;
> >> +    case MKTAG(0xa9,'w','r','t'): key = "writer";    break;
> >> +    case MKTAG(0xa9,'c','o','m'): key = "composer";  break;
> >>     case MKTAG( 'c','p','r','t'):
> >>     case MKTAG(0xa9,'c','p','y'): key = "copyright"; break;
> >>     case MKTAG(0xa9,'g','r','p'): key = "grouping"; break;
> >>     case MKTAG(0xa9,'l','y','r'): key = "lyrics"; break;
> >> -    case MKTAG(0xa9,'c','m','t'):
> >> -    case MKTAG(0xa9,'i','n','f'): key = "comment";   break;
> >> +    case MKTAG(0xa9,'c','m','t'): key = "comment";    break;
> >> +    case MKTAG(0xa9,'i','n','f'): key = "information";break;
> >>     case MKTAG(0xa9,'a','l','b'): key = "album";     break;
> >> -    case MKTAG(0xa9,'d','a','y'): key = "date";      break;
> >> +    case MKTAG(0xa9,'d','a','y'): key = "creation_date";      break;
> >>     case MKTAG(0xa9,'g','e','n'): key = "genre";     break;
> >>     case MKTAG( 'g','n','r','e'): key = "genre";
> >>         parse = mov_metadata_gnre; break;
> >>     case MKTAG(0xa9,'t','o','o'):
> >> -    case MKTAG(0xa9,'s','w','r'): key = "encoder";   break;
> >> +    case MKTAG(0xa9,'s','w','r'): key = "software";   break;
> >>     case MKTAG(0xa9,'e','n','c'): key = "encoder";   break;
> > 
> > with these changes mov -> avi or wav will loose encoder and artist in
> > some cases
> 
> I think I made a mistake with this line
>       case MKTAG(0xa9,'e','n','c'): key = "encoder";   break;
> and think it should have been
>       case MKTAG(0xa9,'e','n','c'): key = "encoded_by";   break;
> "Encoded by" is actually the labelled used for this tag in QuickTime 7. With that change, I would propose this mapping:
> 
> QuickTime:©swr -> RIFF:ISFT
> QuickTime:©enc -> RIFF:ITCH
> 
> Handling artist in mov->riff is harder. Without this patch, QuickTime:©aut and QuickTime:©ART would both map to RIFF:IART and after the patch QuickTime:©aut doesn't have any mapping to RIFF. QuickTime makes a distinction between artist and author, but LIST-INFO doesn't.
> 
> On the other hand, with the current mov.c, if a QuickTime file uses both ©aut and ©ART, then ©aut is used to map to RIFF:IART and ©ART (which seems a better candidate for IART) is ignored. Try: ffmpeg -i http://dericed.com/samples/metadata.mov.
> 
> I think the most authentic mapping would be:
> 
> QuickTime:©aut  doesn't map to RIFF
> QuickTime:©ART -> RIFF:IART
> 
> Thoughts?

RIFF should be changed to take artist if theres one and if none
take author for the IART field


> 
> Also I notice that many of the key values with language modifier get cut off with this patch, for instance:
> special_playbac : specialplaybackrequirements
> 

> From this I found that "key" is declared with a max size of 1024 while "key2" which is used with the language modifier gets a max size of 16 (mov.c line 286). Is there a reason for key2 to be so small?

no idea, i could just guess ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121102/70261861/attachment.asc>