[FFmpeg-devel] [PATCH] asf - read/write metadata as UTF-16

[Anton Khirnov]

On Mon, Feb 22, 2010 at 08:18:53PM +0100, Michael Niedermayer wrote:
>                                           ^^^^^
> unused
> 
oops, fixed
> 
> > +    {\
> > +        uint32_t in = val;\
> > +        if (in < 0x10000) {\
> > +            tmp = in;\
> > +            PUT_16BIT\
> > +        } else if ( in <= 0x10FFFFU ) {\
> 
> else
> 
fixed
>
> > @@ -461,9 +455,11 @@ static int asf_write_header1(AVFormatContext *s, int64_t file_size, int64_t data
> >              put_le16(pb, -1);
> >  
> >          if(enc->codec_id == CODEC_ID_WMAV2)
> > -            put_str16(pb, "Windows Media Audio V8");
> > +            desc = "Windows Media Audio V8";
> >          else
> > -            put_str16(pb, p ? p->name : enc->codec_name);
> > +            desc = p ? p->name : enc->codec_name;
> > +        put_le16(pb, strlen(desc) + 1); // "number of characters" = length in bytes / 2
> 
> the comment and the code dont match
> 
at that point the output is utf-8 interleaved with 0s, so length of
output is exactly 2*strlen(input+1). it gets changed in a later patch
anyway so i don't think writing a more detailed comment is necessary.
>
> > diff --git a/libavformat/asfenc.c b/libavformat/asfenc.c
> > index b24baaf..5f584a2 100644
> > --- a/libavformat/asfenc.c
> > +++ b/libavformat/asfenc.c
> > @@ -345,10 +345,7 @@ static int asf_write_header1(AVFormatContext *s, int64_t file_size, int64_t data
> >          hpos = put_header(pb, &ff_asf_extended_content_header);
> >          put_le16(pb, metadata_count);
> >          while ((tag = av_metadata_get(s->metadata, "", tag, AV_METADATA_IGNORE_SUFFIX))) {
> > -            put_le16(pb, 2*(strlen(tag->key) + 4));
> > -            put_le16(pb, 'W');
> > -            put_le16(pb, 'M');
> > -            put_le16(pb, '/');
> > +            put_le16(pb, 2*(strlen(tag->key) + 1));
> 
> thats storing a different length than it did before
> no doubt the one before was wrong but this doesnt belong in this patch
> and the new looks at least suspicous as thats strlen of utf8
> 
huh? i just removed the 'WM/' prefix => length decreased by 2*3.
> 
> i wouldnt normally be asking but as this is M$ design
> did you check they use bytes/2 for all fields in wmp? :)
> i wouldnt be much surpised if they used that just for some ...
> 
actually they use "number of unicode characters" ( == length/2 ) _only_
for codec name/description in codec list. for everything else they use
length in bytes like almost sane people. tested with wmp, it refuses to
play invalid files.

Anton Khirnov
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-asfdec-read-metadata-as-proper-UTF-16.patch
Type: text/x-diff
Size: 2055 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100222/841ca867/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-asfenc-write-tags-in-proper-UTF-16.patch
Type: text/x-diff
Size: 4212 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100222/841ca867/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-asfenc-simplify-writing-of-comment-header.patch
Type: text/x-diff
Size: 3001 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100222/841ca867/attachment-0002.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-asf-don-t-add-WM-prefix-to-all-tags.patch
Type: text/x-diff
Size: 1753 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100222/841ca867/attachment-0003.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-asfenc-eliminate-put_str16.patch
Type: text/x-diff
Size: 1779 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100222/841ca867/attachment-0004.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-PUT_UTF16-macro.patch
Type: text/x-diff
Size: 1764 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100222/841ca867/attachment-0005.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100222/841ca867/attachment.pgp>