[MPlayer-dev-eng] MPEG audio pass-through fake decoder

[Siarhei Siamashka]

On Sunday 23 July 2006 01:23, Nico Sabbi wrote:

> >'hwmpa' seems to have several problems:
> >1. First it is quite buggy, can crash or hang, and contains some obviously
> >broken code.
>
> I didn't understand that the output of decode_audio() had to be the
> amount of _decoded_ bytes

Well, I'm sorry for probably having confused you. As I get MPlayer code, it is
the amount of bytes that get feeded to 'play' function later. So it is not a
value used for timing and audio/video synchronization, but a size of data
returned from this function.

> >2. Second, its output is not very well suited for audio/video
> > synchronization (or I did not understand synchronization code from
> > ao_mpegpes.c),
>
> tomorrow I will hopefully fix it
>
> >in addition
> >it seems to fail completely when playing audio only.
>
> unrelated: atm ao_mpegpes.c is usable only in combination with vo_mpegpes.c

I understand that, but pass-through fake decoding may be useful not only for
vo_mpegpes.c but for other purposes too (such as mine ;) ) so a single
universal solution working for all output modules can be a good idea.

> >I solved the second problem by padding output data stream so that its size
> >matches the size of decoded data and it seems to work perfectly now.
>
> why padding? because of the inaccuracy of sh->pts while decoding?

Actually I did not invent this myself, the idea is taken here:
http://www.mplayerhq.hu/DOCS/tech/hwac3.txt

This part:
"now i understand how this whole hwac3 mess work.
it's very very tricky. it virtually decodes ac3 to LPCM packets, but really
it keeps the original compressed data padded by zeros. this way it's
constant bitrate, and sync is calculated just like for stereo PCM.
(so it bypass LPCM-capable media converters...)

so, every ac3 frame is translated to 6144 byte long tricky LPCM packet.
6144 = 4*(6*256) = 4 * samples_per_ac3_frame = LPCM size of uncompressed ac3
frame."

> >Unfortunately it introduces incompatibility with current AF_FORMAT_MPEG2
> > and requires a separate fake decoder derived from ad_hwmpeg.c (it can be
> > found in ad_dspmp3.c in my Nokia 770 patch). And also I fixed some of the
> > bugs I could find. Patch against MPlayer-1.0pre8 can be downloaded here:
> >https://garage.maemo.org/frs/?group_id=54
> >
> >MPlayer patched for Nokia 770 still works on a normal desktop PC well (I
> > tried not to break anything). So this gstreamer passthrough stuff can be
> > tested even not having Nokia 770 device (you only need to have
> > gstreamer-0.10 installed with libmad and alsa support).
> >
> >I would like ad_hwmpa.c maintainer to have a look at my ad_dspmp3.c file
> > and take some fixes from it if he finds them useful (mostly mpa_sync()
> > function).
>
> the major difference in mpa_sync() is that you refill the buffer before
> returning
> (but it may not be enough refilled), while I do it in decode_audio(), so
> I didn't commit your change

Well, that's not the issue (though it probably is guaranteed to fill the
buffer unless end of data stream is encountered). The following code has some 
problems:

while(cnt + 4 < sh->a_in_buffer_len)
{
        if(((sh->a_in_buffer[cnt] << 8) | sh->a_in_buffer[cnt+1]) & 0xffe0 != 
0xffe0)
                continue;
        x = mp_get_mp3_header(&(sh->a_in_buffer[cnt]), chans, srate, spf, 
mpa_layer, br);
        if(x > 0)
        {
                frames_count++;
                if(frames_count == no_frames)
                {
                        *n = x;
                        return cnt;
                }
        }
        cnt++;
}

The first problem is a check just after the loop entry, if the condition is
true, the program deadlocks. It can happen when seeking in the audio stream.
The second problem is 1 byte advancement after finding a valid MP3 frame, in
order to get the next frame it could be advanced by 'cnt += x'. We have a
risk of accidently finding a sequence of bytes similar to a MP3 frame header,
also it is not very fast.

> >I know that there are some fixes committed to MPlayer SVN since 1.0pre8
> >release, but from looking at the code, looks like some bugs are still
> > there. Hopefully this could be of any use.
>
> the decode_audio() fix was very useful, but it seems there's a bug in
> your fix:
> memcpy(&buf[cnt2], &(sh->a_in_buffer[start]), len);
> +        cnt2 += chans * spf * 2;
> while cnt2 must be obviously incremented by len alone

This is actually not a bug, but a part of that 'padding' MP3 frames
implementation. Well, I struggled with this pass-through code for slightly
less than a week, so already know quite a number of pitfalls and how to 
avoid them :)

Don't worry. There is no need to make fast decisions and commit fixes in a
hurry, that can lead to more bugs. It is better to check the code carefully
and ensure that everything works.