[FFmpeg-devel] oggparsevp8: set need_context_update when changing codec id

Submitted by Andreas Cadhalpun on Nov. 15, 2016, 9:51 p.m.

Details

Message ID 4954a807-8bbc-194e-3300-7768a56c6935@googlemail.com
State New
Headers show

Commit Message

Andreas Cadhalpun Nov. 15, 2016, 9:51 p.m.
Otherwise the codec context and codecpar might disagree on the codec id,
triggering asserts in av_parser_parse2.

Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
---
 libavformat/oggparsevp8.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Andreas Cadhalpun Nov. 22, 2016, 10:41 p.m.
On 15.11.2016 22:51, Andreas Cadhalpun wrote:
> Otherwise the codec context and codecpar might disagree on the codec id,
> triggering asserts in av_parser_parse2.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
> ---
>  libavformat/oggparsevp8.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/oggparsevp8.c b/libavformat/oggparsevp8.c
> index c534ab1..86495ae 100644
> --- a/libavformat/oggparsevp8.c
> +++ b/libavformat/oggparsevp8.c
> @@ -62,6 +62,7 @@ static int vp8_header(AVFormatContext *s, int idx)
>          st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>          st->codecpar->codec_id   = AV_CODEC_ID_VP8;
>          st->need_parsing      = AVSTREAM_PARSE_HEADERS;
> +        st->internal->need_context_update = 1;
>          break;
>      case 0x02:
>          if (p[6] != 0x20)
> 

Ping. I intend to apply this in a few days.

Best regards,
Andreas
James Almer Nov. 22, 2016, 10:45 p.m.
On 11/15/2016 6:51 PM, Andreas Cadhalpun wrote:
> Otherwise the codec context and codecpar might disagree on the codec id,
> triggering asserts in av_parser_parse2.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
> ---
>  libavformat/oggparsevp8.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libavformat/oggparsevp8.c b/libavformat/oggparsevp8.c
> index c534ab1..86495ae 100644
> --- a/libavformat/oggparsevp8.c
> +++ b/libavformat/oggparsevp8.c
> @@ -62,6 +62,7 @@ static int vp8_header(AVFormatContext *s, int idx)
>          st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>          st->codecpar->codec_id   = AV_CODEC_ID_VP8;
>          st->need_parsing      = AVSTREAM_PARSE_HEADERS;
> +        st->internal->need_context_update = 1;

I'm not sure i understand what's the use for this or how codec_id can be
anything else, but in any case why is this implemented in a different way
than in your oggparsetheora patch?
Andreas Cadhalpun Nov. 22, 2016, 11:23 p.m.
On 22.11.2016 23:45, James Almer wrote:
> On 11/15/2016 6:51 PM, Andreas Cadhalpun wrote:
>> Otherwise the codec context and codecpar might disagree on the codec id,
>> triggering asserts in av_parser_parse2.
>>
>> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
>> ---
>>  libavformat/oggparsevp8.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/libavformat/oggparsevp8.c b/libavformat/oggparsevp8.c
>> index c534ab1..86495ae 100644
>> --- a/libavformat/oggparsevp8.c
>> +++ b/libavformat/oggparsevp8.c
>> @@ -62,6 +62,7 @@ static int vp8_header(AVFormatContext *s, int idx)
>>          st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
>>          st->codecpar->codec_id   = AV_CODEC_ID_VP8;
>>          st->need_parsing      = AVSTREAM_PARSE_HEADERS;
>> +        st->internal->need_context_update = 1;
> 
> I'm not sure i understand what's the use for this

Since the introduction of codecpar, the codec_id of a stream is saved in
two places: the codecpar struct and the internal->avctx context.
These have to agree to avoid undefined behavior.

> or how codec_id can be anything else,

The codec_id can be AV_CODEC_ID_NONE before it is set here.

> but in any case why is this implemented in a different way
> than in your oggparsetheora patch?

The way I used there avoids updating the internal context if
it is not strictly required. I'm not sure which way is better,
as these parsers also set width/height and so on, which should
also match between codecpar and internal context.

Best regards,
Andreas

Patch hide | download patch | download mbox

diff --git a/libavformat/oggparsevp8.c b/libavformat/oggparsevp8.c
index c534ab1..86495ae 100644
--- a/libavformat/oggparsevp8.c
+++ b/libavformat/oggparsevp8.c
@@ -62,6 +62,7 @@  static int vp8_header(AVFormatContext *s, int idx)
         st->codecpar->codec_type = AVMEDIA_TYPE_VIDEO;
         st->codecpar->codec_id   = AV_CODEC_ID_VP8;
         st->need_parsing      = AVSTREAM_PARSE_HEADERS;
+        st->internal->need_context_update = 1;
         break;
     case 0x02:
         if (p[6] != 0x20)