[FFmpeg-devel] [PATCH] BFI demuxer

Sisir Koppaka sisir.koppaka
Sat Apr 12 08:37:40 CEST 2008


Thanks!

On Sat, Apr 12, 2008 at 2:51 AM, Michael Niedermayer <michaelni at gmx.at>
wrote:

> On Sat, Apr 12, 2008 at 12:30:38AM +0530, Sisir Koppaka wrote:
> >
> [...]
> > +    /* Setting total number of frames. */
> > +    url_fseek(pb, 8, SEEK_CUR);
>
> url_fskip()
>
I looked up the definition of url_fskip and it's only a one line function
which again calls url_fseek(pb, x, SEEK_CUR) and hence, to prevent the
unnecessary function call, I put that directly here. Anyway, since you've
pointed out this twice, I've changed it to fskip now, but is there any
specific reason why fskip is preferable to fseek in this case considering
the extra function call overhead?

>
>
> > +    bfi->chunk_header = get_le32(pb);
> > +    bfi->nframes = get_le32(pb);
> > +    bfi->audio_frame = 0;
> > +    bfi->video_frame = 0;
> > +    get_le32(pb);
> > +    get_le32(pb);
> > +    get_le32(pb);
> > +    bfi->fps = get_le32(pb);
> > +    get_le32(pb);
> > +    url_fseek(pb, 44, SEEK_SET);
> > +    bfi->width = get_le32(pb);
> > +    bfi->height = get_le32(pb);
>
> > +    url_fseek(pb, 828, SEEK_SET);
> > +    bfi->sample_rate = get_le32(pb);
> > +    get_le32(pb);
>
> > +    bfi->channels = 1;
>
> unused
>
Removed.

>
>
> > +
> > +    /* Setting up the video codec... */
> > +    vstream = av_new_stream(s, 0);
> > +    if (!vstream)
> > +        return AVERROR(ENOMEM);
> > +
> > +    /*Loading palette to extradata */
> > +    url_fseek(pb, 60, SEEK_SET);
>
> backward seeking
>
Fixed. Removed an unnecessary fseek and changed all current fseeks to
SEEK_CUR, there are no more SEEK_SET's and all reading is in linear fashion
with no backward seeking.

>
>
> > +    vstream->codec->extradata = av_malloc(768);
> > +    vstream->codec->extradata_size = 768;
> > +    get_buffer(pb, vstream->codec->extradata,
> > +               vstream->codec->extradata_size);
> > +
>
> > +    /* Converts the given 6-bit palette values(0-63) to 8-bit
> values(0-255) */
> > +    for (i = 0; i < vstream->codec->extradata_size; i++)
> > +        (vstream->codec->extradata)[i] =
> > +            ((vstream->codec->extradata)[i] << 2) | ((vstream->codec->
> > +                                                      extradata)[i] >>
> 4);
>
> global header convertion does not belong in the demuxer.
>
Fixed. These lines now reside in the decoder.

>
>
> [...]
> > +    /* Setting up the audio codec now... */
> > +    astream = av_new_stream(s, 0);
> > +    if (!astream)
> > +        return AVERROR(ENOMEM);
> > +    astream->codec->codec_type = CODEC_TYPE_AUDIO;
> > +    astream->codec->codec_id = CODEC_ID_PCM_U8;
> > +    astream->codec->sample_rate = bfi->sample_rate;
> > +    av_log(NULL, AV_LOG_DEBUG, "\n sample_rate = %d",
> > +           astream->codec->sample_rate);
> > +    astream->codec->channels = bfi->channels;
>
> > +    bfi->bits_per_sample =
> > +        av_get_bits_per_sample(astream->codec->codec_id);
>
> this can be done simpler
>
Done.

>
>
> > +    astream->codec->bits_per_sample = bfi->bits_per_sample;
> > +    astream->codec->bit_rate = astream->codec->sample_rate *
> astream->codec->bits_per_sample;   // * astream->codec->channels ;
> > +    url_fseek(pb, bfi->chunk_header - 2, SEEK_SET);
> > +    av_set_pts_info(astream, 64, 1, astream->codec->sample_rate);
> > +
> > +    /* Almost done...initialize a flag and a couple of local variables
> and we're done with reading the header. */
> > +    av_log(NULL, AV_LOG_DEBUG,
> > +           "\nFinished reading the headers....starting to read the
> chunks...");
>
> > +    bfi->audio_offset = 0;
> > +    bfi->video_offset = 0;
> > +    bfi->avflag = 0;
>
> The context is automatically initialized to 0
>
:) Sorry about this :) lines deleted...

>
>
> [...]
> > +        /* Trying to confirm the chunk by matching the header... */
> > +      search:
> > +        c = get_byte(pb);
> > +        /* A chunk with a damaged header must also be found...hence,
> the following. */
> > +        if (c == 'I') {
> > +            if (get_byte(pb) == 'V' && get_byte(pb) == 'A'
> > +                && get_byte(pb) == 'S')
> > +                goto read;
> > +        } else if (c == 'V') {
> > +            if (get_byte(pb) == 'A' && get_byte(pb) == 'S')
> > +                goto read;
> > +        } else if (c == 'A') {
> > +            if (get_byte(pb) == 'S')
> > +                goto read;
> > +        } else
> > +            goto search;
>
> infinite loop
>
> Fixed.

>
> [...]
> > +        ret = av_get_packet(pb, pkt, bfi->audio_size);
> > +        if (ret < 0)
> > +            return ret;
> > +        pkt->stream_index = 1;
> > +        pkt->pts = bfi->audio_frame;
> > +        bfi->audio_frame += ret;
>
> > +        pkt->duration = 1;
>
> wrong
>
The duration doesn't seem to be required...i removed the line and it works
fine.

>
>
> > +        av_log(NULL, AV_LOG_DEBUG,
> > +               "\nSending %d bytes to the audio decoder...",
> > +               bfi->audio_size);
> > +        bfi->avflag = 1;
> > +        return ret;
> > +
> > +    case 1:                    //Tossing a video packet at the video
> decoder.
> > +        ret = av_get_packet(pb, pkt, bfi->video_size);
> > +        if (ret < 0)
> > +            return ret;
> > +        pkt->stream_index = 0;
>
> > +        pkt->pts = bfi->video_frame;
>
> > +        bfi->video_frame += ret / bfi->video_size;
> > +        pkt->duration = 1;
>
> these contradict each other
>
Same as above...

-----------------
Sisir Koppaka




More information about the ffmpeg-devel mailing list