From 0882adc69f7a3e420c56f5ba8c4cabb942e285b8 Mon Sep 17 00:00:00 2001 From: Mark Nauwelaerts Date: Fri, 6 Apr 2012 14:23:53 +0200 Subject: [PATCH] h264parse: do not use _unchecked bytewriter variants to insert config .. since the bytewriter is explicitly created with non-fixed size to allow (very much so expected) growth. Partially reverts commit 20669d461a4953eb5ed01374d5ab628b4dc73b64 while trying to keep (some?) compilers happy. IIRC, the purpose of GstByteWriter in the first place was (at least) being able to dump data without having to fuss with memory expansion and size issues ... Fixes #673485. --- gst/h264parse/gsth264parse.c | 13 +++++++++---- gst/videoparsers/gsth264parse.c | 17 +++++++++++------ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/gst/h264parse/gsth264parse.c b/gst/h264parse/gsth264parse.c index 29197371ec..0db2434695 100644 --- a/gst/h264parse/gsth264parse.c +++ b/gst/h264parse/gsth264parse.c @@ -1728,9 +1728,10 @@ gst_h264_parse_push_buffer (GstH264Parse * h264parse, GstBuffer * buf) /* insert config NALs into AU */ GstByteWriter bw; GstBuffer *codec_nal, *new_buf; + gboolean ok; gst_byte_writer_init_with_size (&bw, GST_BUFFER_SIZE (buf), FALSE); - gst_byte_writer_put_data_unchecked (&bw, GST_BUFFER_DATA (buf), + ok = gst_byte_writer_put_data (&bw, GST_BUFFER_DATA (buf), h264parse->idr_offset); GST_DEBUG_OBJECT (h264parse, "- inserting SPS/PPS"); for (i = 0; i < MAX_SPS_COUNT; i++) { @@ -1739,7 +1740,7 @@ gst_h264_parse_push_buffer (GstH264Parse * h264parse, GstBuffer * buf) codec_nal = gst_buffer_copy (h264parse->sps_nals[i]); codec_nal = gst_h264_parse_write_nal_prefix (h264parse, codec_nal); - gst_byte_writer_put_data_unchecked (&bw, + ok &= gst_byte_writer_put_data (&bw, GST_BUFFER_DATA (codec_nal), GST_BUFFER_SIZE (codec_nal)); h264parse->last_report = timestamp; } @@ -1750,12 +1751,12 @@ gst_h264_parse_push_buffer (GstH264Parse * h264parse, GstBuffer * buf) codec_nal = gst_buffer_copy (h264parse->pps_nals[i]); codec_nal = gst_h264_parse_write_nal_prefix (h264parse, codec_nal); - gst_byte_writer_put_data_unchecked (&bw, + ok &= gst_byte_writer_put_data (&bw, GST_BUFFER_DATA (codec_nal), GST_BUFFER_SIZE (codec_nal)); h264parse->last_report = timestamp; } } - gst_byte_writer_put_data_unchecked (&bw, + ok &= gst_byte_writer_put_data (&bw, GST_BUFFER_DATA (buf) + h264parse->idr_offset, GST_BUFFER_SIZE (buf) - h264parse->idr_offset); /* collect result and push */ @@ -1763,6 +1764,10 @@ gst_h264_parse_push_buffer (GstH264Parse * h264parse, GstBuffer * buf) gst_buffer_copy_metadata (new_buf, buf, GST_BUFFER_COPY_ALL); gst_buffer_unref (buf); buf = new_buf; + /* some result checking seems to make some compilers happy */ + if (G_UNLIKELY (!ok)) { + GST_ERROR_OBJECT (h264parse, "failed to insert SPS/PPS"); + } } } } diff --git a/gst/videoparsers/gsth264parse.c b/gst/videoparsers/gsth264parse.c index 0c8455d49d..af0afac56c 100644 --- a/gst/videoparsers/gsth264parse.c +++ b/gst/videoparsers/gsth264parse.c @@ -1489,17 +1489,18 @@ gst_h264_parse_pre_push_frame (GstBaseParse * parse, GstBaseParseFrame * frame) GstByteWriter bw; GstBuffer *new_buf; const gboolean bs = h264parse->format == GST_H264_PARSE_FORMAT_BYTE; + gboolean ok; gst_byte_writer_init_with_size (&bw, GST_BUFFER_SIZE (buffer), FALSE); - gst_byte_writer_put_data_unchecked (&bw, GST_BUFFER_DATA (buffer), + ok = gst_byte_writer_put_data (&bw, GST_BUFFER_DATA (buffer), h264parse->idr_pos); GST_DEBUG_OBJECT (h264parse, "- inserting SPS/PPS"); for (i = 0; i < GST_H264_MAX_SPS_COUNT; i++) { if ((codec_nal = h264parse->sps_nals[i])) { GST_DEBUG_OBJECT (h264parse, "inserting SPS nal"); - gst_byte_writer_put_uint32_be_unchecked (&bw, + ok &= gst_byte_writer_put_uint32_be (&bw, bs ? 1 : GST_BUFFER_SIZE (codec_nal)); - gst_byte_writer_put_data_unchecked (&bw, + ok &= gst_byte_writer_put_data (&bw, GST_BUFFER_DATA (codec_nal), GST_BUFFER_SIZE (codec_nal)); h264parse->last_report = new_ts; } @@ -1507,14 +1508,14 @@ gst_h264_parse_pre_push_frame (GstBaseParse * parse, GstBaseParseFrame * frame) for (i = 0; i < GST_H264_MAX_PPS_COUNT; i++) { if ((codec_nal = h264parse->pps_nals[i])) { GST_DEBUG_OBJECT (h264parse, "inserting PPS nal"); - gst_byte_writer_put_uint32_be_unchecked (&bw, + ok &= gst_byte_writer_put_uint32_be (&bw, bs ? 1 : GST_BUFFER_SIZE (codec_nal)); - gst_byte_writer_put_data_unchecked (&bw, + ok &= gst_byte_writer_put_data (&bw, GST_BUFFER_DATA (codec_nal), GST_BUFFER_SIZE (codec_nal)); h264parse->last_report = new_ts; } } - gst_byte_writer_put_data_unchecked (&bw, + ok &= gst_byte_writer_put_data (&bw, GST_BUFFER_DATA (buffer) + h264parse->idr_pos, GST_BUFFER_SIZE (buffer) - h264parse->idr_pos); /* collect result and push */ @@ -1525,6 +1526,10 @@ gst_h264_parse_pre_push_frame (GstBaseParse * parse, GstBaseParseFrame * frame) GST_BUFFER_FLAG_UNSET (new_buf, GST_BUFFER_FLAG_DELTA_UNIT); gst_buffer_replace (&frame->buffer, new_buf); gst_buffer_unref (new_buf); + /* some result checking seems to make some compilers happy */ + if (G_UNLIKELY (!ok)) { + GST_ERROR_OBJECT (h264parse, "failed to insert SPS/PPS"); + } } } /* we pushed whatever we had */