[FFmpeg-soc] [Patch] Maxis EA XA decoder - GSoC Task

[Robert Marston]

On Fri, Apr 11, 2008 at 8:18 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> On Fri, Apr 11, 2008 at 04:19:37PM +0000, compn wrote:
>  > Michael Niedermayer <michaelni at ...> writes:
>  >
>  > >
>  > > On Fri, Apr 11, 2008 at 03:41:48PM +0200, Robert Marston wrote:
>  > > > >  > > > +                    sample = ((((*(src+channel) >> i) & 0x0F) <<
>  > 0x1C) >> shift[channel]);
>  > > > >  > >
>  > > > >  > >  This looks buggy.
>  > > > >  >
>  > > > >  > Where do you think the error would occur?
>  > > > >
>  > > > >  on some non x86 hardware
>  > > >
>  > > > What exactly you referring to here? The shift operators? A problem
>  > > > with the Endianess maybe?
>  > >
>  > > Its related to the shifts.
>  >
>  >
>  > i dont mean to be rude, but i believe that Robert may need some more verbose
>  > output from you :)
>
>  Good C knowledge is a requirement for ffmpeg gsoc.
>  If one uses a strict interpretation of the C standard then there are multiple
>  bugs in the line above. Iam not doing that, i just mean one actual issue
>  which can occur on real hardware.
>
>  Heres an example to help a little
>  *(src+channel)= 234
>  i= 4
>  shift[channel]= 20
>  sample=  -512 (normlly)
>  sample=  3584 (on ILP64)
>

Thanks for pointing that out, I will be the first to admit that my c
knowledge is probably not up to standard when it comes to FFMPEG and
as such would required much feedback from my mentor. I see the GSoC as
good learning opportunity for the myself and a chance to bolster open
source development and potential to increase the code base of the
mentor organizations project.

Would casting the *(src + channel) to a int32_t stop the above from happening?

>
>  >
>  > > > >  > >  Still wrong
>  > > > >  > Corrected?
>  > > > >  no
>  > > > Am I right in saying the pts should be incremented by 28 *
>  > > no
>  >
>  >
>  > more verbosity needed here too.
>
>  Iam not sure what verbosity over
>
>  typedef struct AVPacket {
>     int64_t pts;                            ///< presentation time stamp in time_base units
>
>  is needed? Also there is the source of other demuxers and the source of
>  the code using pts.
>  Robert has an application for H.264 SVC (H.264 is one of the most complex
>  codecs around) how is he planing to understand the existing code and
>  integrate SVC support if he cannot figure out how to set the timestamps
>  for an adpcm codec?
>
>  [...]

My logic on this is that there are 2 samples per byte and 14 bytes per
channel. = 28 x num_channels
There are is 1 sample every 1 / sample rate seconds and 90 K ticks per
second if we use a 90 KHz clock

Therefore the pts, which in my understanding of the time base being
the number of ticks of the 90 KHz clock, will be advanced  by 28 x
num_channels /  sample rate * 90 K for each block read from the file.

Have I misunderstood something here?