[FFmpeg-devel] [PATCH] ffmpeg doesn't decode track number on some wma files

[Patrice Bensoussan]

On 14 Jun 2007, at 20:58, Michael Niedermayer wrote:

> Hi
>
> On Thu, Jun 14, 2007 at 07:20:18PM +0100, Patrice Bensoussan wrote:
>>
>> On 14 Jun 2007, at 09:52, Michael Niedermayer wrote:
>>
>>> Hi
>>>
>>> On Thu, Jun 14, 2007 at 09:13:52AM +0100, Patrice Bensoussan wrote:
>>>>
>>>> On 14 Jun 2007, at 09:00, Benoit Fouet wrote:
>>>>
>>>>> Patrice Bensoussan wrote:
>>>>>> Hello,
>>>>>>
>>>>>> On 14 Jun 2007, at 08:40, Benoit Fouet wrote:
>>>>>>
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Patrice Bensoussan wrote:
>>>>>>>
>>>>>>>> Index: libavformat/asf.c
>>>>>>>> =============================================================== 
>>>>>>>> ==
>>>>>>>> ==
>>>>>>>> --- libavformat/asf.c	(revision 9304)
>>>>>>>> +++ libavformat/asf.c	(working copy)
>>>>>>>> @@ -389,6 +389,16 @@
>>>>>>>>                         {
>>>>>>>>                                 if     (!strcmp(name,"WM/
>>>>>>>> AlbumTitle")) get_str16_nolen(pb, value_len, s->album, sizeof 
>>>>>>>> (s-
>>>>>>>>
>>>>>>>>> album));
>>>>>>>>>
>>>>>>>>                                 else if(!strcmp(name,"WM/
>>>>>>>> Genre"     )) get_str16_nolen(pb, value_len, s->genre, sizeof 
>>>>>>>> (s-
>>>>>>>>
>>>>>>>>> genre));
>>>>>>>>>
>>>>>>>> +                                else if (!strcmp(name,"WM/
>>>>>>>> Track")) {
>>>>>>>> +                                    char track[8];
>>>>>>>> +                                    get_str16_nolen(pb,
>>>>>>>> value_len, track, sizeof(track));
>>>>>>>> +                                    s->track = strtol(track,
>>>>>>>> NULL, 10) + 1;
>>>>>>>> +                                }
>>>>>>>> +                                else if (!strcmp(name,"WM/
>>>>>>>> TrackNumber")) {
>>>>>>>> +                                    char track[8];
>>>>>>>> +                                    get_str16_nolen(pb,
>>>>>>>> value_len, track, sizeof(track));
>>>>>>>> +                                    s->track = strtol(track,
>>>>>>>> NULL, 10);
>>>>>>>> +                                }
>>>>>>>>
>>>>>>>>
>>>>>>> you should try to keep the nice alignment
>>>>>>> also, if i read correctly, it is the very same code for the two
>>>>>>> else if
>>>>>>> you added. Why not add them into a single one ?
>>>>>>>
>>>>>>
>>>>>> The alignment seems to be correct for me (I checked with an hex
>>>>>> editor and can only see spaces unless I am missing something).
>>>>>
>>>>> i was more thinking of if and strcmp's parenthesis alignment
>>>>>
>>>>
>>>> OK, I missed that one. Here is a new patch with the if parenthesis
>>>> aligned.
>>> [...]
>>>> Index: libavformat/asf.c
>>>> ===================================================================
>>>> --- libavformat/asf.c	(revision 9304)
>>>> +++ libavformat/asf.c	(working copy)
>>>> @@ -389,6 +389,16 @@
>>>>                          {
>>>>                                  if     (!strcmp(name,"WM/
>>>> AlbumTitle")) get_str16_nolen(pb, value_len, s->album, sizeof(s-
>>>>> album));
>>>>                                  else if(!strcmp(name,"WM/
>>>> Genre"     )) get_str16_nolen(pb, value_len, s->genre, sizeof(s-
>>>>> genre));
>>>> +                                else if(!strcmp(name,"WM/ 
>>>> Track")) {
>>>> +                                    char track[8];
>>>> +                                    get_str16_nolen(pb,
>>>> value_len, track, sizeof(track));
>>>> +                                    s->track = strtol(track,
>>>> NULL, 10) + 1;
>>>> +                                }
>>>> +                                else if(!strcmp(name,"WM/
>>>> TrackNumber")) {
>>>> +                                    char track[8];
>>>> +                                    get_str16_nolen(pb,
>>>> value_len, track, sizeof(track));
>>>> +                                    s->track = strtol(track,
>>>> NULL, 10);
>>>> +                                }
>>>
>>> what about
>>> char buf[123];
>>> get_str16_nolen(pb, value_len, track, sizeof(buf));
>>> if(     !strcmp(name,"WM/AlbumTitle")) copy
>>> else if(!strcmp(name,"WM/Genre"     )) copy
>>> else if(!strcmp(name,"WM/Track"     )) s->track = strtol(track,
>>> NULL, 10) + 1;
>>> ...
>>>
>>> ?
>>
>> This code would be slower (you are going to do some extra copying and
>
> this is used only by the header reading code
>
>
>> I think it is less readable IMHO. I would rather stick with the patch
>> submitted.
>
> well i have no strong oppinion on this, do whatever you like, asf  
> is a mess
> anyway
>

The patch is good enough for me... If anyone wants to commit...
Patrice