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

[Michael Niedermayer]

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

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

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070614/19fdf524/attachment.pgp>