[FFmpeg-devel] [PATCH] Bitstream filter DTS from any format to 14bit format.

[Måns Rullgård]

Bartlomiej Wolowiec <bartek.wolowiec at gmail.com> writes:

> Tuesday 04 August 2009 22:29:55 M?ns Rullg?rd napisa?(a):
>> Bartlomiej Wolowiec <bartek.wolowiec at gmail.com> writes:
>> > Hi,
>> > Here is a small bitstream filter, which may be useful for someone.
>> > --
>> > Bartlomiej Wolowiec
>>
>> [...]
>> > +
>> > +    *poutbuf_size = ((dca_buffer_size + 13) / 14) << 4;
>> > +    *poutbuf = av_malloc(*poutbuf_size + FF_INPUT_BUFFER_PADDING_SIZE);
>> > +    pout = (uint16_t *) * poutbuf;
>>
>> Doesn't gcc complain about strict aliasing violations here?
>
> Hmm... gcc-4.3.2 and gcc-4.1.2 with -Wstrict-aliasing=2 don't show any 
> warnings. Any suggestions ?

Never mind.  The source is a pointer to uint8_t, so aliasing is OK.

>> > +
>> > +    while (((uint8_t *) pout - *poutbuf) & 7)
>> > +        *(pout++) = 0;
>>
>> What's all this padding for?
>
> New packet should start at first bit of byte, but also should start at first 
> bit of 14bits word.



[...]

> +static int dts14b_filter(AVBitStreamFilterContext * bsfc,
> +                         AVCodecContext * avctx, const char *args,
> +                         uint8_t ** poutbuf, int *poutbuf_size,
> +                         const uint8_t * buf, int buf_size, int keyframe)
> +{
> +    GetBitContext gb;
> +    int dca_buffer_size;
> +    uint8_t dca_buffer[DCA_MAX_FRAME_SIZE];
> +    uint16_t *pout;
> +
> +    dca_buffer_size =
> +        ff_dca_convert_bitstream(buf, buf_size, dca_buffer,
> +                                 DCA_MAX_FRAME_SIZE);

Would it be worthwhile to check if the source is already in the
desired format, or shall we assume the user wouldn't ask for
conversion in that case?

> +    if (dca_buffer_size == -1) {
> +        av_log(avctx, AV_LOG_ERROR, "Not a valid DCA frame\n");
> +        return -1;
> +    }
> +
> +    *poutbuf_size = ((dca_buffer_size + 13) / 14) << 4;

Shouldn't this be (((dca_buffer_size << 3) + 13) / 14) << 1, or am I
missing some trick?

> +    *poutbuf = av_malloc(*poutbuf_size + FF_INPUT_BUFFER_PADDING_SIZE);
> +    pout = (uint16_t *) * poutbuf;
                            ^
Please drop that space -----+ like this:

+    pout = (uint16_t *) *poutbuf

It's not multiplication...

> +    init_get_bits(&gb, dca_buffer, dca_buffer_size);
> +    while (get_bits_count(&gb) < dca_buffer_size << 3)

This can over-read the end of the input buffer.  You should add a few
bytes padding to be safe.

> +        *pout++ = be2me_16(get_sbits(&gb, 14));
> +
> +    while (((uint8_t *) pout - *poutbuf) & 7)
> +        *pout++ = 0;

Shouldn't the tail padding be included in the returned size?

> +    return 0;
> +}

-- 
M?ns Rullg?rd
mans at mansr.com