[FFmpeg-devel] [PATCH] h264 bitstream filter
Benoit Fouet
benoit.fouet
Fri Aug 31 10:58:46 CEST 2007
M?ns Rullg?rd wrote:
> Benoit Fouet <benoit.fouet at purplelabs.com> writes:
>
>
>> Michael Niedermayer wrote:
>>
>>> Hi
>>>
>>> On Thu, Aug 30, 2007 at 05:12:09PM +0200, Benoit Fouet wrote:
>>> [...]
>>>
>>>
>>>> updated patch attached
>>>>
>>>>
>>> [...]
>>>
>>>
>>>> + bsfc->priv_data = out;
>>>> + bsfc->filter->priv_data_size = priv_data_size;
>>>>
>>>>
>>> bsfc->filter is shared between all instances
>>> writing to it is absolutely not allowed
>>>
>>> please properly initalize priv_data_size to the size of your context
>>> and store whatever you need in this context
>>>
>>>
>>>
>> it's always easier after some sleep :)
>> here it is, along with other modifications i thought of...
>>
>> Index: libavcodec/Makefile
>> ===================================================================
>> --- libavcodec/Makefile (revision 10260)
>> +++ libavcodec/Makefile (working copy)
>> @@ -326,6 +326,7 @@
>> OBJS-$(CONFIG_MP3_HEADER_DECOMPRESS_BSF) += mp3_header_decompress_bsf.o mpegaudiodata.o
>> OBJS-$(CONFIG_MJPEGA_DUMP_HEADER_BSF) += mjpega_dump_header_bsf.o
>> OBJS-$(CONFIG_IMX_DUMP_HEADER_BSF) += imx_dump_header_bsf.o
>> +OBJS-$(CONFIG_MP4_AVC_TO_AVC_ES_BSF) += mp4_avc_to_avc_es_bsf.o
>>
>
> Diego sorted these so you'll need to update this.
>
>
all fixed.
>> +static void alloc_and_copy( uint8_t **poutbuf, int *poutbuf_size,
>> + const uint8_t *in1, uint32_t in1_size,
>> + const uint8_t *in2, uint32_t in2_size,
>> + uint32_t nal_offset) {
>> + *poutbuf = av_malloc(in1_size+in2_size);
>> + *poutbuf_size = in1_size+in2_size;
>> + memcpy(*poutbuf, in1, in1_size);
>> + if (in2)
>> + memcpy(*poutbuf+in1_size, in2, in2_size);
>> + (*poutbuf)[nal_offset ] = 0;
>> + (*poutbuf)[nal_offset+1] = 0;
>> + (*poutbuf)[nal_offset+2] = 0;
>> + (*poutbuf)[nal_offset+3] = 1;
>>
>
> Maybe use AV_WB32 there.
>
>
i'll have a look
>> +}
>> +
>> +static int mp4_avc_to_avc_es( AVBitStreamFilterContext *bsfc,
>> + AVCodecContext *avctx, const char *args,
>> + uint8_t **poutbuf, int *poutbuf_size,
>> + const uint8_t *buf, int buf_size,
>> + int keyframe ) {
>> + /* nothing to filter */
>> + if (!avctx->extradata || buf_size < 5 || avctx->extradata_size < 9) {
>> + *poutbuf = (uint8_t*) buf;
>> + *poutbuf_size = buf_size;
>> + return 0;
>> + }
>> +
>> + else {
>>
>
> No need for the else when you return in the if. Save yourself a level
> of indentation.
>
>
true, fixed
>> + if (unit_type == 5 /* IDR picture */)
>> + /* sps and pps have to be prepended to NAL unit */
>> + alloc_and_copy(poutbuf, poutbuf_size,
>> + ctx->sps_pps_data, ctx->size,
>> + buf, buf_size,
>> + ctx->size);
>> + else if (unit_type == 6 /* SEI */ || unit_type == 9 /* AUD */)
>> + /* sps and pps have to be appended to NAL unit */
>> + alloc_and_copy(poutbuf, poutbuf_size,
>> + buf, buf_size,
>> + ctx->sps_pps_data, ctx->size,
>> + 0);
>>
>
> This is wrong. Maybe you misunderstood what I said about where to
> insert SPS and PPS.
it seems so...
> They should be added before the first type 5 NAL
> unit of an IDR picture, after whatever SEI or AUD units that picture
> has. A multislice IDR picture only needs SPS and PPS before the first
> slice, and non-IDR pictures should not have SPS or PPS added.
>
>
i'm not sure i get it, here is what i understood, please correct me if
i'm wrong
i receive NALU in the filter
if it's the first coded slice of an IDR picture (type 5 NAL), i prepend
sps and pps NALUs (i don't really have to care about SEI or AUD in the
bitstream filter case, right?)
else, i don't add sps and pps
this would give, in pseudo code:
if (not sps and pps data)
retrieve sps and pps
first_idr=1
if (first_idr && nal_unit_type == 5)
prepend sps and pps
first_idr=0
else
don't prepend sps and pps
if( nal_unit_type != 5)
first_idr=1
is this right ?
>> + else
>> + alloc_and_copy(poutbuf, poutbuf_size,
>> + buf, buf_size,
>> + NULL, 0,
>> + 0);
>> + }
>> +
>> + return 1;
>> +}
>> +
>> +AVBitStreamFilter mp4_avc_to_avc_es_bsf = {
>> + "mp4_avc_to_avc_es",
>>
>
> I don't like that name.
>
>
neither do i, i'm open if someone has a better one...
>> + sizeof( AVCBSFContext ),
>>
>
> I'm allergic to spaces inside parenthesis.
>
>
fixed locally
--
Ben
Purple Labs S.A.
www.purplelabs.com
More information about the ffmpeg-devel
mailing list