| Message ID | 20201208194330.2962-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [FFmpeg-devel,1/4] avcodec/h264_sei: refactor parsing User Data Registered ITU-T T35 SEI messages | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On Tue, Dec 8, 2020 at 9:44 PM James Almer <jamrial@gmail.com> wrote: > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavcodec/h264_sei.c | 31 ++++++++++++++++++++++--------- > 1 file changed, 22 insertions(+), 9 deletions(-) Set generally OK, just some minor things: - probably AVERROR(EINVAL) usage should be AVERROR_INVALIDDATA ? - Unrelated to this specific set (but just visible in the context of the change), but I see that we seem to have generally not handled the case of country_code == 0xFF && size == 7 (or 3). I think while we're refactoring this code we could just add a "if (size < 1) return AVERROR_INVALIDDATA;" into that country_code == 0xFF check? Should make it handle that specific case now. Jan
On 12/18/2020 12:51 PM, Jan Ekström wrote: > On Tue, Dec 8, 2020 at 9:44 PM James Almer <jamrial@gmail.com> wrote: >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavcodec/h264_sei.c | 31 ++++++++++++++++++++++--------- >> 1 file changed, 22 insertions(+), 9 deletions(-) > > Set generally OK, just some minor things: > - probably AVERROR(EINVAL) usage should be AVERROR_INVALIDDATA ? > - Unrelated to this specific set (but just visible in the context of > the change), but I see that we seem to have generally not handled the > case of country_code == 0xFF && size == 7 (or 3). I think while we're > refactoring this code we could just add a "if (size < 1) return > AVERROR_INVALIDDATA;" into that country_code == 0xFF check? Should > make it handle that specific case now. > > Jan Both things changed, and set pushed. Thanks.
diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c index 669560ae5f..42ba94114c 100644 --- a/libavcodec/h264_sei.c +++ b/libavcodec/h264_sei.c @@ -183,12 +183,11 @@ static int decode_registered_user_data_closed_caption(H264SEIA53Caption *h, static int decode_registered_user_data(H264SEIContext *h, GetBitContext *gb, void *logctx, int size) { - uint32_t country_code; - uint32_t user_identifier; + int country_code, provider_code; - if (size < 7) + if (size < 3) return AVERROR_INVALIDDATA; - size -= 7; + size -= 3; country_code = get_bits(gb, 8); // itu_t_t35_country_code if (country_code == 0xFF) { @@ -196,20 +195,34 @@ static int decode_registered_user_data(H264SEIContext *h, GetBitContext *gb, size--; } + if (country_code != 0xB5) // usa_country_code + return 0; + /* itu_t_t35_payload_byte follows */ - skip_bits(gb, 8); // terminal provider code - skip_bits(gb, 8); // terminal provider oriented code - user_identifier = get_bits_long(gb, 32); + provider_code = get_bits(gb, 16); + + switch (provider_code) { + case 0x31: { // atsc_provider_code + uint32_t user_identifier; - switch (user_identifier) { + if (size < 4) + return AVERROR(EINVAL); + size -= 4; + + user_identifier = get_bits_long(gb, 32); + switch (user_identifier) { case MKBETAG('D', 'T', 'G', '1'): // afd_data return decode_registered_user_data_afd(&h->afd, gb, size); case MKBETAG('G', 'A', '9', '4'): // closed captions return decode_registered_user_data_closed_caption(&h->a53_caption, gb, logctx, size); default: - skip_bits(gb, size * 8); break; + } + break; + } + default: + break; } return 0;
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/h264_sei.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-)