[FFmpeg-devel] [PATCH] H.264/AVCHD interlaced fixes
Michael Niedermayer
michaelni
Fri Feb 13 02:20:27 CET 2009
On Tue, Feb 10, 2009 at 09:36:35PM +0100, Ivan Schreter wrote:
> Hi Michael,
>
> Michael Niedermayer wrote:
>
>>>> the parsing and combining are seperate things requireing seperate
>>>> patches.
>>>>
> OK, so here the parsing-only patch.
[...]
> #6a is the parsing of additional information from H.264 picture to get key
> frame flag, recovery count, field picture flag and frame number out of the
> stream in parser for latter use in lavf. Currently, only key frame flag can
> be transported via picture type, so recovery count and field picture flag
> are not yet communicated. These will be needed, though, in implementation
> of correct timestamps (and/or frame combining).
>
> So please review #6a.
>
> Regards,
>
> Ivan
>
>
[...]
> Index: libavcodec/h264_parser.c
> ===================================================================
> --- libavcodec/h264_parser.c (revision 17130)
> +++ libavcodec/h264_parser.c (working copy)
> @@ -27,6 +27,8 @@
>
> #include "parser.h"
> #include "h264_parser.h"
> +#include "h264data.h"
> +#include "golomb.h"
>
> #include <assert.h>
>
> @@ -96,6 +98,167 @@
> return i-(state&5);
> }
>
> +/*!
> + * Parse NAL units of found picture and decode some basic information.
> + *
> + * @param s parser context.
> + * @param avctx codec context.
> + * @param buf buffer with field/frame data.
> + * @param buf_size size of the buffer.
> + * @param ppict_type set to picture type.
> + * @param pfield_pict set to 1 for field picture, 0 for frame picture.
> + * @param pkey_frame set to 1, if key frame found (IDR or SEI recovery point).
> + * @param precovery_frame_cnt set to recovery frame count for key frames
> + * (if SEI recovery point present), otherwise set to -1.
> + * @param pframe_num set to frame number.
> + */
> +static int parse_nal_units(AVCodecParserContext *s,
> + AVCodecContext *avctx,
> + const uint8_t *buf, int buf_size,
> + int *ppict_type,
> + int *pfield_pict,
> + int *pkey_frame,
> + int *precovery_frame_cnt,
> + int *pframe_num)
> +{
> + H264Context *h = s->priv_data;
> + const uint8_t *buf_end= buf + buf_size;
> + int pps_id;
> + int slice_type;
> + const uint8_t *ptr;
> +
> + /* set some sane values */
> + *ppict_type = FF_I_TYPE;
> + *precovery_frame_cnt = -1;
> + *pkey_frame = 0;
> + *pfield_pict = 0;
> +
> + h->s.avctx= avctx;
> + h->sei_recovery_frame_cnt = -1;
> + for(;;){
> + int dst_length, consumed, bit_length;
> + for(; buf + 3 < buf_end; buf++){
> + // This should always succeed in the first iteration.
> + if(buf[0] == 0 && buf[1] == 0 && buf[2] == 1)
> + break;
> + }
> + buf += 3;
> + if(buf >= buf_end)
> + break;
> + ptr= ff_h264_decode_nal(h, buf, &dst_length, &consumed, buf_end - buf);
this effectively reads through the whole bitstream
obviously this is not ok and unneeded
> + if (ptr==NULL || dst_length < 0)
> + break;
> + while(ptr[dst_length - 1] == 0 && dst_length > 0)
> + dst_length--;
> + bit_length= !dst_length ? 0 : (8*dst_length - ff_h264_decode_rbsp_trailing(h, ptr + dst_length - 1));
> +
> + init_get_bits(&h->s.gb, ptr, bit_length);
> + switch(h->nal_unit_type){
> + case NAL_SPS:
> + ff_h264_decode_seq_parameter_set(h);
> + break;
> + case NAL_PPS:
> + ff_h264_decode_picture_parameter_set(h, h->s.gb.size_in_bits);
> + break;
> + case NAL_SEI:
> + ff_h264_decode_sei(h);
> + break;
> + case NAL_IDR_SLICE:
> + case NAL_SLICE:
> + get_ue_golomb(&h->s.gb); // skip first_mb_in_slice
> + slice_type = get_ue_golomb_31(&h->s.gb) % 5;
> + *ppict_type= golomb_to_pict_type[slice_type];
> + *precovery_frame_cnt = h->sei_recovery_frame_cnt;
> + if (h->nal_unit_type == NAL_IDR_SLICE) {
> + /* this is an IDR key frame (mostly at begin of the stream) */
> + *pkey_frame = 1;
> + } else if (h->sei_recovery_frame_cnt >= 0) {
> + /* key frame, since recovery_frame_cnt is set */
> + *pkey_frame = 1;
> + }
> + pps_id= get_ue_golomb(&h->s.gb);
> + if(pps_id>=MAX_PPS_COUNT){
> + av_log(h->s.avctx, AV_LOG_ERROR, "pps_id out of range\n");
> + return -1;
> + }
> + if(!h->pps_buffers[pps_id]) {
> + av_log(h->s.avctx, AV_LOG_ERROR, "non-existing PPS referenced\n");
> + return -1;
> + }
> + h->pps= *h->pps_buffers[pps_id];
> + if(!h->sps_buffers[h->pps.sps_id]) {
> + av_log(h->s.avctx, AV_LOG_ERROR, "non-existing SPS referenced\n");
> + return -1;
> + }
> + h->sps = *h->sps_buffers[h->pps.sps_id];
> + *pframe_num = get_bits(&h->s.gb, h->sps.log2_max_frame_num);
> + if (h->sps.frame_mbs_only_flag) {
> + /* single picture for a frame */
> + *pfield_pict = 0;
> + } else {
> + if (get_bits1(&h->s.gb)) { /* field_pict_flag */
> + /*
> + * This frame is divided in two separate field pictures.
> + * Field parity bit follows in picture header, but we
> + * don't need it.
> + */
> + *pfield_pict = 1;
> + } else {
> + *pfield_pict = 0;
> + }
> + }
split the patch please so that
IDR-keyframe
SEI recovery
field pics
are in 3 seperate patches
the original patch i wrote was not such an intermingled mix
[...]
> +/**
> + * Set context parameters for lavf.
> + *
> + * @param s parser context.
> + * @param pict_type picture type of the first slice in frame.
> + * @param field_pict set to 1 for field picture, 0 for frame picture.
> + * @param key_frame set to 1 for key pictures, 0 for non-key pictures.
> + * @param recovery_frame_cnt frame count from SEI recovery point if present
> + * or -1 otherwise.
> + * @param frame_num frame number.
> + */
> +static inline void h264_set_keyframe(AVCodecParserContext *s,
> + int pict_type,
> + int field_pict,
> + int key_frame,
> + int recovery_frame_cnt,
> + int frame_num)
> +{
> + /*
> + * NOTE: Currently, there is no flag to tell libavformat about
> + * key frames. Instead, it relies on having pict_type set to I
> + * for key frames, so we use this to communicate it. This should
if your patch contains a hack like this its rejected.
if theres a bug it has to be fixed, not worked around by such hacks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- 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/20090213/b17f7939/attachment.pgp>
More information about the ffmpeg-devel
mailing list