[FFmpeg-devel] [PATCH] oma demuxer

[Michael Niedermayer]

On Fri, May 30, 2008 at 02:09:35PM +0200, Benjamin Larsson wrote:
> Michael Niedermayer wrote:
> > 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 :)

well you can also remove get_atrac3_info() completely and inline the code if you
prefer.


[...]
> static int is_ea3_tag_header(const uint8_t *buf)
> {
>     return (!memcmp(buf, (uint8_t[]){'e', 'a', '3', 3, 0},5));
> }

i think it would be cleaner to replace the single use of this by the
actual code


[...]
>     /* 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;
>         default:
>             *sample_rate = 0;
>     }
> }
[...]
>     /* 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;
>     }

if(((info >> 13) & 7) != 2) {
    ...
}
st->codec->sample_rate= 44100;


> 
>     /* try to detect atrac3 mode using framesize */
>     switch (framesize) {
>         case 192: bit_rate=66000;  break;
>         case 272: bit_rate=94000;  break;
>         case 304: bit_rate=105000; break;
>         case 384: bit_rate=132000; break;
>         default:
>             av_log(s, AV_LOG_INFO, "Invalid frame size: %d\n", framesize);
>             return -1;
>     }

these numbers contradict themselfs
bit_rate= sample_rate * framesize * 8 / 1024

we do not need a switch to set false values.


[...]
> static int oma_read_packet(AVFormatContext *s, AVPacket *pkt)
> {
>     int ret, size;
> 
>     size = s->streams[0]->codec->block_align;
> 
>     ret= av_get_packet(s->pb, pkt, size);

the size variable is redundant.


> 
>     pkt->stream_index = 0;
>     if (ret <= 0) {
>         return AVERROR(EIO);
>     }

superflous {}


> 
>     return ret;
> }
> 

> static int oma_read_seek(AVFormatContext *s,
>                          int stream_index, int64_t timestamp, int flags)
> {
>     return pcm_read_seek(s, stream_index, timestamp, flags);
> }

you can use pcm_read_seek below directly if above is correct


> 
> AVInputFormat oma_demuxer = {
>     "oma",
>     "Sony OpenMG audio",
>     0,
>     oma_read_probe,
>     oma_read_header,
>     oma_read_packet,
>     NULL,
>     oma_read_seek,
>     .flags= AVFMT_GENERIC_INDEX,
>     .extensions = "oma,aa3",
> };
> 
[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato
-------------- 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/20080530/5010ce17/attachment.pgp>