momo0853 / mp4v2

mp4v2 for android
Other
19 stars 20 forks source link

Name and mean tags like iTunEXTC are not shown in iTunes #127

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
Hi,

I tried to tag mp4-files with name-tags like iTunEXTC. The tags are copied to 
the file, but iTunes does not recognize them. When I imported the tagged file 
in iTunes, iTunes doesn't show the age rating information.

I used the following code segment:

                MP4ItmfItem* item = MP4ItmfItemAlloc( "----", 1 );
                bogus->mean = strdup( "com.apple.iTunes" );
                bogus->name = strdup( "iTunEXTC" );
                const char* const fsk = "de-movie|FSK 18|500|";
                MP4ItmfData* data = &item->dataList.elements[0];
                data->typeCode = MP4_ITMF_BT_UTF8;
                data->valueSize = strlen( fsk );
                data->value = (uint8_t*)malloc( data->valueSize );
                memcpy( data->value, fsk, data->valueSize );
                bool result = MP4ItmfAddItem( destFile, item );

I think, that the name atoms are not stored in the correct format, but thats 
only a guess.

Version 1.9.0, Mac OS X Lion 10.7.2

If needed, I can send you the tagged file to reproduce the error in iTunes. It 
is to big for attachment. 

Thanks

Holger

Original issue reported on code.google.com by holgr.ja...@googlemail.com on 20 Dec 2011 at 10:46

GoogleCodeExporter commented 8 years ago
Hi,

ok, after two days of investigation I found the problem: the property values of 
the mean and name tags are written as null terminated string. The '\0' 
character at the end is included in the atom. That's an issue, because iTunes 
expect a fixed length string without terminating zero. 

I fixed it in generic.cpp by setting the fixed length parameter to the actual 
sting length:

static bool
__itemModelToAtom( const MP4ItmfItem& model, MP4ItemAtom& atom )
{
    if( ATOMID( atom.GetType() ) == ATOMID( "----" )) {
        ASSERT( model.mean ); // mandatory
        MP4MeanAtom& meanAtom = *(MP4MeanAtom*)MP4Atom::CreateAtom( &atom, "mean" );
        atom.AddChildAtom( &meanAtom );
        meanAtom.value.SetValue( model.mean );
>   meanAtom.value.SetFixedLength( strlen(model.mean) );

        if( model.name ) {
            MP4NameAtom& nameAtom = *(MP4NameAtom*)MP4Atom::CreateAtom( &atom, "name" );
            atom.AddChildAtom( &nameAtom );
            nameAtom.value.SetValue( model.name );
>       nameAtom.value.SetFixedLength( strlen(model.name) );
        }
    }

    for( uint32_t i = 0; i < model.dataList.size; i++ ) {
...

Original comment by holgr.ja...@googlemail.com on 5 Jan 2012 at 8:30

GoogleCodeExporter commented 8 years ago
I'll work on getting this change in today; if I understand you correctly, 
you're basically just telling the atom the length of the value, so existing 
code that expected a NULL terminator would still work correctly?

Original comment by kid...@gmail.com on 5 Jan 2012 at 7:02

GoogleCodeExporter commented 8 years ago
Actually, sorry--have you tried trunk, or the current developer release?  The 
code that's currently there is significantly different from what's in 1.9.0 
(which, actually, I'd advise against using because there's a ton of known 
issues):

static bool
__itemModelToAtom( const MP4ItmfItem& model, MP4ItemAtom& atom )
{
    if( ATOMID( atom.GetType() ) == ATOMID( "----" )) {
        ASSERT( model.mean ); // mandatory
        MP4MeanAtom& meanAtom = *(MP4MeanAtom*)MP4Atom::CreateAtom( atom.GetFile(), &atom, "mean" );
        atom.AddChildAtom( &meanAtom );
        meanAtom.value.SetValue( (const uint8_t*)model.mean, (uint32_t)strlen( model.mean ));

        if( model.name ) {
            MP4NameAtom& nameAtom = *(MP4NameAtom*)MP4Atom::CreateAtom( atom.GetFile(), &atom, "name" );
            atom.AddChildAtom( &nameAtom );
            nameAtom.value.SetValue( (const uint8_t*)model.name, (uint32_t)strlen( model.name ));
        }
    } 

I want to do a release soon, and now might be as good a time as any if this is 
a bug we can fix (or establish as fixed).

Original comment by kid...@gmail.com on 5 Jan 2012 at 7:13

GoogleCodeExporter commented 8 years ago
Closing this bug; already fixed in trunk.  Please let me know if you have an 
issue w/trunk and I'll revisit the issue.

Original comment by kid...@gmail.com on 19 Mar 2012 at 2:37