[FFmpeg-devel] [PATCH] oma demuxer

Michael Niedermayer michaelni
Sat May 24 17:42:01 CEST 2008


On Sat, May 24, 2008 at 03:55:23PM +0200, Benjamin Larsson wrote:
> Michael Niedermayer wrote:
> > On Sat, May 24, 2008 at 04:51:10AM +0200, Benjamin Larsson wrote:
> >> Michael Niedermayer wrote:
> > [...]
[...]
> > 
> >> }
> >>
> >>
> > 
> >> static void get_atrac3_info(const uint8_t *buf, int *frame_size, int *mode, int *sample_rate)
> >> {
> > 
> > AVCodecContext could be passed as arg instead of these dozen pointers.
> > 
> 
> Well there is no context left.

there still is AVCodecContext, really, you are storing things in it :)


[...]
> > 
> > 
> >>     /* get framesize */
> >>     /* soundUnit * 4 * nChannels */
> >>     *frame_size = (info & 0x3FF) * 8;
> > 
> > the *8 could be moved to where block_align is inited, this would also allow
> > uint8_t to be used i the table ...
> > 
> 
> Fixed. But everything is now obfuscated, before it was possible to
> understand the table by looking at it.

we could write 192/8 and such in the table?


> 
> > 
> >>     /* get coding mode */
> >>     *mode = (info >> 17) & 1;
> >>
> >>     /* decode sample rate */
> >>     switch ((info >> 13) & 7) {
> > 
> >>         case 0:
> >>             *sample_rate = 32000;
> >>             break;
> >>         case 1:
> >>             *sample_rate = 44100;
> >>             break;
> >>         case 2:
> >>             *sample_rate = 48000;
> >>             break;
> > 
> > ff_ac3_sample_rate_tab could be used here
> > 
> 
> It's in the wrong order and 3 bits=6 elements can overflow the table. In
> the future there might be other sample rates.

i meant

case 0:
case 1:
case 2:
    *sample_rate = ff_ac3_sample_rate_tab[ ...


but anyway leave it, its becoming too messy


[...]

> static struct {
>     uint8_t    bitrate;

kbitrate


>     uint8_t    framesize;
>     uint8_t    coding_mode;
> }  atrac3_modes[5] = {
>     {66,  24, 1},
>     {94,  34, 1},
>     {105, 38, 0},
>     {132, 48, 0},
>     {  0,  0, 0},
> };
> 
> 

> static int is_ea3_tag_header(const uint8_t *buf)
> {
>     return (!memcmp(buf, "ea3", 3) && buf[3] == 3 && buf[4] == 0);
> }

i meant

s/is_ea3_tag_header(buf)/memcmp(buf, (uint8_t[]){'e', 'a', '3', 3, 0})/


[...]
>     /* check the EA3 header */
>     if (strncmp(buf, "EA3", 3) || buf[4] != 0 || buf[5] != EA3_HEADER_SIZE) {
>         av_log(s, AV_LOG_INFO, "Couldn't find the EA3 header !\n");
>         return -1;
>     }

could use memcmp() too



> 
>     /* check for encrypted content */
>     eid = AV_RB16(&buf[6]);
>     if (eid != -1 && eid != -128) {
>         av_log(s, AV_LOG_INFO, "Encrypted file! Eid: %d\n", eid);
>         return -1;
>     }
> 
>     /* check for codec support */
>     codec_id = buf[32];
>     if (codec_id != 0) {
>         av_log(s, AV_LOG_INFO, "Sorry, unsupported codec! id: %d\n", codec_id);
>         return -1;
>     }
> 
>     /* get the atrac3 codec parameters */
>     get_atrac3_info(buf, &framesize, &jsflag, &samplerate);
>     if (samplerate != 44100) {
>         av_log(s, AV_LOG_INFO, "Unsupported sample rate, send sample file to developers: %d\n", samplerate);
>         return -1;
>     }
> 

>     /* try to detect atrac3 mode using framesize */
>     for (mode=0 ; mode<5 ; mode++) {

mode<4 and the table does not need a 5th 0,0,0 entry

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080524/a1d4ba34/attachment.pgp>



More information about the ffmpeg-devel mailing list