[Mplayer-dev-eng] AC3 output.

[Arpi]

Hi,

> If there is not other way to search the mailing list archive could you
> please tell me the main reasons for you refusing his patch? I don't want
> to make similar mistakes :-)
here are: 

> > In short:
> > - AC3 passthrough code shouldn't be in libac3.
> >   libac3 is a software decoder. just memcpy'ing soem buyes shouldn't be
> >   there.
> 	
> 	Well, in fact I suppose that libac3 is the decoder library for
> AC3, so when you need to decode it partially (in fact only reorder some
> headers to make it IEC985 compliant) it should be there also, shouldn't
> it? I mean I can change two lines in the decode_frame in order to make
> AC3 through spdif work, just skip everything but the search for sync and
> header building.
i saw that patch.. search for sync can be written is a few lines of code.

'header building' means putting a few bytes to teh audio_out buffer.
can be easily made in dec_audio.c

> >   It should go to dec_audio.c. There is 2 coices:
> >    - adding to PCM codec (it already has some support)
> >    - adding a new codec, for example HWAC3
> >   A simple AC3 frame sync code can be written in 10 lines. I see no
> >   sense of using libac3 for this purpose.
> >   Btw I think that AC3 stream shouldn't be synced and hacked to work,
> >   it's enough to send ac3 starting code once, before the playback starts,
> >   and then send the ac3 stream unmodified. So no extra code required.
> >   (must be tested, i have no external AC3 decoder)
> 	
> 	Uhmm, what happen if you fast forward or rewind the stream?
dec_audio.c:resync_audio_stream() will be called, so you can handle
this, and re-send header bytes if needed.
but i'm sure that ac3 decoder hardware does the syncing itself, as almost
every other decoder chips.

> > - sh_audio->o_bps must be fixed (must be set to compressed ac3 bytes /
> > sec)
> >   to avoid A-V desync and buffer overflow.
> > - as you said too, a control() function should be added to ao_oss.c, to
> >   query/set AC3 support.
> > - you have to #ifdef AFMT_AC3 constant everywhere, because it isn't
> > defined
> >   on non-linux systems, and in older linux kernel includes (old
> > soundcard.h)
> 
> 	That was what I was thinking: just change the two lines in the,
> decode_frame. In the main.c check for the -spdif option, and check if
> /dev/dsp is AFMT_AC3 capable, if it is set some global variable, then
> change the two lines in decode_frame so it doesn't "decode" anything
> and make some changes in ao_oss to initialize the card correctly and 
> write the raw data, really simple and quite easy to review :-)
and it's a very big messy hack.
i'm on cleaning up mplayer.c and separate stuff, to make it possible
to re-use parts of code in other projects, and make it possible to
build a (or more) GUI replacing mplayer.c with other code.
and you comes and hack up everything with your globals...

you can do that, but your patch won't be commited to CVS then.
sorry.

mplayer finally has it's layered structure, please don't mess it up.
place things to the right layer, even if it requires few more lines to type.

for example, mplayer.c shouldn't know about what type of audio card is used.
it's handled in libao2. mplayer.c (and other modules) see only a general
interface suitable for every card/driver type.

codec dependent stuff must be in dec_*.c files.
and don't add new commandline option, as codec selection is already written:
-ac <codecname>, codecs are defined in codecs.conf.
you can add new codec _driver_ if you want, or use the existing PCM or AC3.


A'rpi / Astral & ESP-team

--
mailto:arpi at thot.banki.hu
http://esp-team.scene.hu

_______________________________________________
Mplayer-dev-eng mailing list
Mplayer-dev-eng at lists.sourceforge.net
http://lists.sourceforge.net/lists/listinfo/mplayer-dev-eng