[FFmpeg-devel] [PATCH] AVCHD/H.264 parser: determination of frame type, question about timestamps
Michael Niedermayer
michaelni
Mon Jan 19 02:30:25 CET 2009
On Sun, Jan 18, 2009 at 10:13:34AM +0100, Ivan Schreter wrote:
> Hi avcoder,
>
> avcoder wrote:
>> Dear:
>>
>> The patch is full of TAB
>>
>>> Ivan Schreter wrote:
>>>
>>>> And here the patch for h264_parser.c:
>>>>
>
> sorry, but I noticed too late that TABs are not used in the code. I tried
> to remove them all, but I missed two initial TABs (and two in comments).
>
> Attached is a corrected TAB-less version.
>
> However, I'd like to hear an opinion about the algorithmic side (see my
> original post). Any takers?
[...]
> + if (h->is_avc && h->nal_length_size < 1)
> + return -1; /* AVC not inited yet */
> +
> + while (buf < buf_end) {
> + int nalsize = 0;
> + int buf_index = 0;
> + int i;
> + uint8_t nal_hdr;
> + uint8_t nal_type;
> + int hdrsize = 1;
> +
> + if (h->is_avc) {
there seems to be no code ever setting this, and no there is no need
for a decoder to be there ...
[...]
> + } else {
> + /* start code prefix search */
> + for (; buf_index + 3 < buf_size; buf_index++){
> + /* this should always succeed in the first iteration */
> + if(buf[buf_index] == 0 && buf[buf_index+1] == 0 && buf[buf_index+2] == 1)
> + break;
> + }
> +
> + if (buf_index + 3 >= buf_size)
> + break;
> +
> + buf_index += 3;
> + }
> +
> + buf += buf_index;
> +
> + /* NAL header 1b zero, 2b nal_ref_idc, 5b nal_unit_type */
> + nal_hdr = *buf;
> + nal_type = nal_hdr & 0x1f;
> + if (nal_hdr & 0x80) {
> + av_log(h->s.avctx, AV_LOG_ERROR, "invalid NAL header (%x)\n", nal_hdr);
> + return -1;
> + }
> + if (nal_type == 14 || nal_type == 20)
> + hdrsize += 3;
> + buf += hdrsize;
> +
> + if (nal_type == NAL_SLICE) {
> + /* Decode picture type. It contains slice type in second
> + * variable-sized integer in buffer */
> + GetBitContext ctx;
> + int slice_type;
> + init_get_bits(&ctx, buf, 8 * (buf_end - buf));
> + /*first_mb_in_slice =*/ get_ue_golomb_31(&ctx);
> + slice_type = get_ue_golomb_31(&ctx);
> + if (slice_type > 9) {
> + av_log(h->s.avctx, AV_LOG_ERROR, "slice type too large (%d)\n", slice_type);
> + return -1;
> + }
> + if (slice_type > 4)
> + slice_type -= 5;
> +
> + slice_type = golomb_to_pict_type[slice_type];
> + s->pict_type= slice_type;
> + break; /* no more data necessary, save some time */
> + }
you are duplicating h264.c, dont do this
and if you now argue you just need this little bit, no you need 50 times
more for the timestamps
my current code for cleaning things up is below, feel free to use it
for your future patches, i seem to be too lazy to work on it :(
Index: libavcodec/h264.c
===================================================================
--- libavcodec/h264.c (revision 16492)
+++ libavcodec/h264.c (working copy)
@@ -7020,7 +7020,7 @@
}
}
-static inline int decode_seq_parameter_set(H264Context *h){
+int ff_h264_decode_seq_parameter_set(H264Context *h){
MpegEncContext * const s = &h->s;
int profile_idc, level_idc;
unsigned int sps_id;
@@ -7165,7 +7165,7 @@
pps->chroma_qp_table[t][i] = chroma_qp[av_clip(i + index, 0, 51)];
}
-static inline int decode_picture_parameter_set(H264Context *h, int bit_length){
+int ff_h264_decode_picture_parameter_set(H264Context *h, int bit_length){
MpegEncContext * const s = &h->s;
unsigned int pps_id= get_ue_golomb(&s->gb);
PPS *pps;
@@ -7462,7 +7462,7 @@
break;
case NAL_SPS:
init_get_bits(&s->gb, ptr, bit_length);
- decode_seq_parameter_set(h);
+ ff_h264_decode_seq_parameter_set(h);
if(s->flags& CODEC_FLAG_LOW_DELAY)
s->low_delay=1;
@@ -7473,7 +7473,7 @@
case NAL_PPS:
init_get_bits(&s->gb, ptr, bit_length);
- decode_picture_parameter_set(h, bit_length);
+ ff_h264_decode_picture_parameter_set(h, bit_length);
break;
case NAL_AUD:
Index: libavcodec/h264_parser.c
===================================================================
--- libavcodec/h264_parser.c (revision 16492)
+++ libavcodec/h264_parser.c (working copy)
@@ -27,6 +27,7 @@
#include "parser.h"
#include "h264_parser.h"
+#include "golomb.h"
#include <assert.h>
@@ -96,6 +97,39 @@
return i-(state&5);
}
+static void parse_nal_units(AVCodecParserContext *s,
+ AVCodecContext *avctx,
+ const uint8_t *buf, int buf_size)
+{
+ H264Context *h = s->priv_data;
+ uint32_t state=-1;
+ uint8_t *buf_end= buf + buf_size;
+
+
+ h->s.avctx= avctx;
+ s->pict_type= FF_I_TYPE;
+ while(buf<buf_end){
+ buf= ff_find_start_code(buf, buf_end, &state);
+ init_get_bits(&h->s.gb, buf, 8*(buf_end - buf));
+ switch(state & 0x1F){
+ 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_IDR_SLICE:
+ case NAL_SLICE:
+ get_ue_golomb(&h->s.gb);
+ if(get_ue_golomb(&h->s.gb) % 5 == 2)
+ s->pict_type= FF_I_TYPE;
+ else
+ s->pict_type= FF_P_TYPE;
+ return;
+ }
+ }
+}
+
static int h264_parse(AVCodecParserContext *s,
AVCodecContext *avctx,
const uint8_t **poutbuf, int *poutbuf_size,
@@ -122,6 +156,8 @@
}
}
+ parse_nal_units(s, avctx, buf, buf_size);
+
*poutbuf = buf;
*poutbuf_size = buf_size;
return next;
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- 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/20090119/13cfddce/attachment.pgp>
More information about the ffmpeg-devel
mailing list