From patchwork Wed May 22 09:56:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 13244 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id B45E1444A8D for ; Wed, 22 May 2019 12:57:01 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 846C9689ADE; Wed, 22 May 2019 12:57:01 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 3DEF9680282 for ; Wed, 22 May 2019 12:56:55 +0300 (EEST) Received: by mail-wm1-f68.google.com with SMTP id 15so1502077wmg.5 for ; Wed, 22 May 2019 02:56:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=aRuaLKE2CBynTvbssycAb+rgGtrSfFFJRf+mzUlsETc=; b=Q3ao1xN4G/vkXYL7ija7vzS7/yZVaIq3PbHfmQPNAp03GaqTzPFfD+ObINT2thlaWx OYXDOrrxmJM56qw86YZfzzwSuZQHaBOilHDji690eomES7gTyXuDJ/RJRu9P1IY8w6lS ja+0HqW3ie9l0LVia+MtQQT8zJtyG8VyB/ENmwOzjPuVvfCvW4lzQYEGIRlRefWpIerU i0oeB0WYm6kfnTMeYdAr0kEIw+ozD9W6alyt3cRficJCIXmgQ4sQDNZlu684tcE6RxA+ 6xow7bUtlo4gPPO2PKBz5qCXQi/qPKrOnngO3JLlPKM80Ihg7J2osL6jW6l8eo1KnoWK 46NA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=aRuaLKE2CBynTvbssycAb+rgGtrSfFFJRf+mzUlsETc=; b=Etiq2Oz+GeWwmJlPmIgXIHk0snI6KPd4vqYOFx0Rm3A4IdwW1VfQupsWbMWqBqGMf9 hU3E6jJwVVAj/HHWAeQO65F+pQ9lg+xW1AS1sZ111CSv25p1PZxOeEG7D1SF89x/6Rvl 9FaY+WCKUiKR65kvJ1Bd4RiZ2xaKYQc+JMMQSWMGYoBSyGs9XT/hqDI3IGCp3WLGeq9a v4wvRucyxYIRf2iOZzitaI31jx/pp2Nz9EfaUfWEeizpDvuJO20He9oVAXvzoHHC5ACr MeKqLpw007C+KOuVG703aTlSzAtyVCOhxSakcJiXBF+PgXafJ7cCDdYKWpLvtayEVZGp Camw== X-Gm-Message-State: APjAAAW250UOa1XyMjRw86kzzJCcyqnHWobeViyK6qVr04er7ECjjBM6 jUqEwEu2BXrJFmIHb6dM/87Mj2eI X-Google-Smtp-Source: APXvYqwM1IdC4+D+6Kpz7UyvPMVQlwTBeKGtlTA1uHnkcadlca16OcUvQYPimKfGZLkIGjgImpmJbA== X-Received: by 2002:a1c:9904:: with SMTP id b4mr7010270wme.1.1558519014456; Wed, 22 May 2019 02:56:54 -0700 (PDT) Received: from localhost.localdomain (ipbcc18715.dynamic.kabel-deutschland.de. [188.193.135.21]) by smtp.gmail.com with ESMTPSA id 88sm57629835wrc.33.2019.05.22.02.56.53 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 22 May 2019 02:56:53 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Wed, 22 May 2019 11:56:01 +0200 Message-Id: <20190522095601.47148-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190522010441.44257-11-andreas.rheinhardt@gmail.com> References: <20190522010441.44257-11-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH] cbs_mpeg2: Fix parsing of picture headers X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" MPEG-2 picture and slice headers can contain optional extra information; both use the same syntax for their extra information. And cbs_mpeg2's implementations of both were buggy until recently; the one for the picture headers still is and this is fixed in this commit. The extra information in picture headers has simply been forgotten. This meant that if this extra information was present, it was discarded during reading; and unfortunately writing created invalid bitstreams in this case (an extra_bit_picture - the last set bit of the whole unit - indicated that there would be a further byte of data, although the output didn't contain said data). This has been fixed; both types of extra information are now parsed via the same code and essentially passed through. Signed-off-by: Andreas Rheinhardt --- New version made necessary because of a merge conflict due to James' request. libavcodec/cbs_mpeg2.c | 31 +++++++----- libavcodec/cbs_mpeg2.h | 12 +++-- libavcodec/cbs_mpeg2_syntax_template.c | 66 +++++++++++++++----------- 3 files changed, 66 insertions(+), 43 deletions(-) diff --git a/libavcodec/cbs_mpeg2.c b/libavcodec/cbs_mpeg2.c index eea0186c97..f7283f6363 100644 --- a/libavcodec/cbs_mpeg2.c +++ b/libavcodec/cbs_mpeg2.c @@ -41,18 +41,18 @@ #define SUBSCRIPTS(subs, ...) (subs > 0 ? ((int[subs + 1]){ subs, __VA_ARGS__ }) : NULL) #define ui(width, name) \ - xui(width, name, current->name, 0, MAX_UINT_BITS(width), 0) + xui(width, #name, current->name, 0, MAX_UINT_BITS(width), 0) #define uir(width, name) \ - xui(width, name, current->name, 1, MAX_UINT_BITS(width), 0) + xui(width, #name, current->name, 1, MAX_UINT_BITS(width), 0) #define uis(width, name, subs, ...) \ - xui(width, name, current->name, 0, MAX_UINT_BITS(width), subs, __VA_ARGS__) + xui(width, #name, current->name, 0, MAX_UINT_BITS(width), subs, __VA_ARGS__) #define uirs(width, name, subs, ...) \ - xui(width, name, current->name, 1, MAX_UINT_BITS(width), subs, __VA_ARGS__) + xui(width, #name, current->name, 1, MAX_UINT_BITS(width), subs, __VA_ARGS__) #define sis(width, name, subs, ...) \ - xsi(width, name, current->name, MIN_INT_BITS(width), MAX_INT_BITS(width), subs, __VA_ARGS__) + xsi(width, #name, current->name, MIN_INT_BITS(width), MAX_INT_BITS(width), subs, __VA_ARGS__) #define marker_bit() \ - bit(marker_bit, 1) + bit("marker_bit", 1) #define bit(name, value) do { \ av_unused uint32_t bit = value; \ xui(1, name, bit, value, value, 0); \ @@ -65,7 +65,7 @@ #define xui(width, name, var, range_min, range_max, subs, ...) do { \ uint32_t value; \ - CHECK(ff_cbs_read_unsigned(ctx, rw, width, #name, \ + CHECK(ff_cbs_read_unsigned(ctx, rw, width, name, \ SUBSCRIPTS(subs, __VA_ARGS__), \ &value, range_min, range_max)); \ var = value; \ @@ -73,7 +73,7 @@ #define xsi(width, name, var, range_min, range_max, subs, ...) do { \ int32_t value; \ - CHECK(ff_cbs_read_signed(ctx, rw, width, #name, \ + CHECK(ff_cbs_read_signed(ctx, rw, width, name, \ SUBSCRIPTS(subs, __VA_ARGS__), &value, \ range_min, range_max)); \ var = value; \ @@ -103,13 +103,13 @@ #define RWContext PutBitContext #define xui(width, name, var, range_min, range_max, subs, ...) do { \ - CHECK(ff_cbs_write_unsigned(ctx, rw, width, #name, \ + CHECK(ff_cbs_write_unsigned(ctx, rw, width, name, \ SUBSCRIPTS(subs, __VA_ARGS__), \ var, range_min, range_max)); \ } while (0) #define xsi(width, name, var, range_min, range_max, subs, ...) do { \ - CHECK(ff_cbs_write_signed(ctx, rw, width, #name, \ + CHECK(ff_cbs_write_signed(ctx, rw, width, name, \ SUBSCRIPTS(subs, __VA_ARGS__), var, \ range_min, range_max)); \ } while (0) @@ -136,6 +136,13 @@ #undef infer +static void cbs_mpeg2_free_picture_header(void *unit, uint8_t *content) +{ + MPEG2RawPictureHeader *picture = (MPEG2RawPictureHeader*)content; + av_buffer_unref(&picture->extra_information_picture.extra_information_ref); + av_free(content); +} + static void cbs_mpeg2_free_user_data(void *unit, uint8_t *content) { MPEG2RawUserData *user = (MPEG2RawUserData*)content; @@ -146,7 +153,7 @@ static void cbs_mpeg2_free_user_data(void *unit, uint8_t *content) static void cbs_mpeg2_free_slice(void *unit, uint8_t *content) { MPEG2RawSlice *slice = (MPEG2RawSlice*)content; - av_buffer_unref(&slice->header.extra_information_ref); + av_buffer_unref(&slice->header.extra_information_slice.extra_information_ref); av_buffer_unref(&slice->data_ref); av_free(content); } @@ -249,7 +256,7 @@ static int cbs_mpeg2_read_unit(CodedBitstreamContext *ctx, } \ break; START(MPEG2_START_PICTURE, MPEG2RawPictureHeader, - picture_header, NULL); + picture_header, &cbs_mpeg2_free_picture_header); START(MPEG2_START_USER_DATA, MPEG2RawUserData, user_data, &cbs_mpeg2_free_user_data); START(MPEG2_START_SEQUENCE_HEADER, MPEG2RawSequenceHeader, diff --git a/libavcodec/cbs_mpeg2.h b/libavcodec/cbs_mpeg2.h index db5ebc56ea..2befaab275 100644 --- a/libavcodec/cbs_mpeg2.h +++ b/libavcodec/cbs_mpeg2.h @@ -114,6 +114,12 @@ typedef struct MPEG2RawGroupOfPicturesHeader { uint8_t broken_link; } MPEG2RawGroupOfPicturesHeader; +typedef struct MPEG2RawExtraInformation { + uint8_t *extra_information; + AVBufferRef *extra_information_ref; + size_t extra_information_length; +} MPEG2RawExtraInformation; + typedef struct MPEG2RawPictureHeader { uint8_t picture_start_code; @@ -126,7 +132,7 @@ typedef struct MPEG2RawPictureHeader { uint8_t full_pel_backward_vector; uint8_t backward_f_code; - uint8_t extra_bit_picture; + MPEG2RawExtraInformation extra_information_picture; } MPEG2RawPictureHeader; typedef struct MPEG2RawPictureCodingExtension { @@ -194,9 +200,7 @@ typedef struct MPEG2RawSliceHeader { uint8_t slice_picture_id_enable; uint8_t slice_picture_id; - size_t extra_information_length; - uint8_t *extra_information; - AVBufferRef *extra_information_ref; + MPEG2RawExtraInformation extra_information_slice; } MPEG2RawSliceHeader; typedef struct MPEG2RawSlice { diff --git a/libavcodec/cbs_mpeg2_syntax_template.c b/libavcodec/cbs_mpeg2_syntax_template.c index 1f2d2497c3..325a48676e 100644 --- a/libavcodec/cbs_mpeg2_syntax_template.c +++ b/libavcodec/cbs_mpeg2_syntax_template.c @@ -173,6 +173,40 @@ static int FUNC(group_of_pictures_header)(CodedBitstreamContext *ctx, RWContext return 0; } +static int FUNC(extra_information)(CodedBitstreamContext *ctx, RWContext *rw, + MPEG2RawExtraInformation *current, + const char *element_name, const char *marker_name) +{ + int err; + size_t k; +#ifdef READ + GetBitContext start = *rw; + uint8_t bit; + + for (k = 0; nextbits(1, 1, bit); k++) + skip_bits(rw, 1 + 8); + current->extra_information_length = k; + if (k > 0) { + *rw = start; + current->extra_information_ref = + av_buffer_allocz(k + AV_INPUT_BUFFER_PADDING_SIZE); + if (!current->extra_information_ref) + return AVERROR(ENOMEM); + current->extra_information = current->extra_information_ref->data; + } +#endif + + for (k = 0; k < current->extra_information_length; k++) { + bit(marker_name, 1); + xui(8, element_name, + current->extra_information[k], 0, 255, 1, k); + } + + bit(marker_name, 0); + + return 0; +} + static int FUNC(picture_header)(CodedBitstreamContext *ctx, RWContext *rw, MPEG2RawPictureHeader *current) { @@ -197,7 +231,8 @@ static int FUNC(picture_header)(CodedBitstreamContext *ctx, RWContext *rw, ui(3, backward_f_code); } - ui(1, extra_bit_picture); + CHECK(FUNC(extra_information)(ctx, rw, ¤t->extra_information_picture, + "extra_information_picture[k]", "extra_bit_picture")); return 0; } @@ -369,33 +404,10 @@ static int FUNC(slice_header)(CodedBitstreamContext *ctx, RWContext *rw, ui(1, intra_slice); ui(1, slice_picture_id_enable); ui(6, slice_picture_id); - - { - size_t k; -#ifdef READ - GetBitContext start; - uint8_t bit; - start = *rw; - for (k = 0; nextbits(1, 1, bit); k++) - skip_bits(rw, 1 + 8); - current->extra_information_length = k; - if (k > 0) { - *rw = start; - current->extra_information_ref = - av_buffer_allocz(k + AV_INPUT_BUFFER_PADDING_SIZE); - if (!current->extra_information_ref) - return AVERROR(ENOMEM); - current->extra_information = current->extra_information_ref->data; - } -#endif - for (k = 0; k < current->extra_information_length; k++) { - bit(extra_bit_slice, 1); - xui(8, extra_information_slice[k], - current->extra_information[k], 0, 255, 1, k); - } - } } - bit(extra_bit_slice, 0); + + CHECK(FUNC(extra_information)(ctx, rw, ¤t->extra_information_slice, + "extra_information_slice[k]", "extra_bit_slice")); return 0; }