Re: [FFmpeg-devel] [PATCH v2 3/3] lavc/libdavs2: correct frame type setting
At 2018-11-01 08:40:50, "Carl Eugen Hoyos" wrote:
>2018-10-31 23:15 GMT+01:00, Mark Thompson :
>> On 31/10/18 10:23, hwren wrote:
>
>>> +switch (pic->type) {
>>> +case DAVS2_PIC_I:
>>> +frame->pict_type = AV_PICTURE_TYPE_I;
>>> +break;
>>> +case DAVS2_PIC_P:
>>> +frame->pict_type = AV_PICTURE_TYPE_P;
>>> +break;
>>> +case DAVS2_PIC_B:
>>> +case DAVS2_PIC_F:
>>> +frame->pict_type = AV_PICTURE_TYPE_B;
>>> +break;
>>> +default:
>>> +frame->pict_type = AV_PICTURE_TYPE_NONE;
>>
>> Are there any types which aren't already handled? If there aren't
>> then this would probably be better as an assert.
>
>Since this is an external library, an assert would be wrong.
>
>I wanted to suggest an error message or a negative return value.
Actually, there are six types of frames defined by AVS2 standard:
DAVS2_PIC_I , DAVS2_PIC_P, DAVS2_PIC_B
DAVS2_PIC_G, DAVS2_PIC_F ,DAVS2_PIC_S
(the DAVS2_PIC_S is not exactly the same thing as AV_PICTURE_TYPE_S)
While in this patch I may make a wrong classification. Every frame in davs2 has
its same/similar type in ffmpeg.
So maybe it's better to set the unhandled-type to an error assert.
Thanks :-)
>
>Carl Eugen
>___
>ffmpeg-devel mailing list
>ffmpeg-devel@ffmpeg.org
>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2 3/3] lavc/libdavs2: correct frame type setting
2018-10-31 23:15 GMT+01:00, Mark Thompson :
> On 31/10/18 10:23, hwren wrote:
>> +switch (pic->type) {
>> +case DAVS2_PIC_I:
>> +frame->pict_type = AV_PICTURE_TYPE_I;
>> +break;
>> +case DAVS2_PIC_P:
>> +frame->pict_type = AV_PICTURE_TYPE_P;
>> +break;
>> +case DAVS2_PIC_B:
>> +case DAVS2_PIC_F:
>> +frame->pict_type = AV_PICTURE_TYPE_B;
>> +break;
>> +default:
>> +frame->pict_type = AV_PICTURE_TYPE_NONE;
>
> Are there any types which aren't already handled? If there aren't
> then this would probably be better as an assert.
Since this is an external library, an assert would be wrong.
I wanted to suggest an error message or a negative return value.
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2 3/3] lavc/libdavs2: correct frame type setting
On 31/10/18 10:23, hwren wrote:
> Signed-off-by: hwren
> ---
> libavcodec/libdavs2.c | 17 -
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
> index a1815d2..d7bcaa3 100644
> --- a/libavcodec/libdavs2.c
> +++ b/libavcodec/libdavs2.c
> @@ -94,11 +94,26 @@ static int davs2_dump_frames(AVCodecContext *avctx,
> davs2_picture_t *pic,
> pic->widths[plane] * bytes_per_sample);
> }
>
> +switch (pic->type) {
> +case DAVS2_PIC_I:
> +frame->pict_type = AV_PICTURE_TYPE_I;
> +break;
> +case DAVS2_PIC_P:
> +frame->pict_type = AV_PICTURE_TYPE_P;
> +break;
> +case DAVS2_PIC_B:
> +case DAVS2_PIC_F:
> +frame->pict_type = AV_PICTURE_TYPE_B;
> +break;
> +default:
> +frame->pict_type = AV_PICTURE_TYPE_NONE;
Are there any types which aren't already handled? If there aren't then this
would probably be better as an assert.
> +}
> +
> frame->width = cad->headerset.width;
> frame->height= cad->headerset.height;
> frame->pts = cad->out_frame.pts;
> -frame->pict_type = pic->type;
> frame->format= avctx->pix_fmt;
> +frame->key_frame = pic->type == DAVS2_PIC_I ? 1 : 0;
Is an I frame in AVS2 necessarily a random access point? (That is, not like an
I frame in H.264, instead like an IDR frame - it clears the DPB so no previous
reference frames can be carried across it.)
>
> return 1;
> }
>
Thanks
- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH v2 3/3] lavc/libdavs2: correct frame type setting
2018-10-31 11:23 GMT+01:00, hwren :
> Signed-off-by: hwren
> ---
> libavcodec/libdavs2.c | 17 -
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
> index a1815d2..d7bcaa3 100644
> --- a/libavcodec/libdavs2.c
> +++ b/libavcodec/libdavs2.c
> @@ -94,11 +94,26 @@ static int davs2_dump_frames(AVCodecContext *avctx,
> davs2_picture_t *pic,
> pic->widths[plane] * bytes_per_sample);
> }
>
> +switch (pic->type) {
> +case DAVS2_PIC_I:
> +frame->pict_type = AV_PICTURE_TYPE_I;
> +break;
> +case DAVS2_PIC_P:
> +frame->pict_type = AV_PICTURE_TYPE_P;
> +break;
> +case DAVS2_PIC_B:
> +case DAVS2_PIC_F:
> +frame->pict_type = AV_PICTURE_TYPE_B;
> +break;
> +default:
> +frame->pict_type = AV_PICTURE_TYPE_NONE;
What other DAVS2_PIC values exist?
Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH v2 3/3] lavc/libdavs2: correct frame type setting
Signed-off-by: hwren
---
libavcodec/libdavs2.c | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/libavcodec/libdavs2.c b/libavcodec/libdavs2.c
index a1815d2..d7bcaa3 100644
--- a/libavcodec/libdavs2.c
+++ b/libavcodec/libdavs2.c
@@ -94,11 +94,26 @@ static int davs2_dump_frames(AVCodecContext *avctx,
davs2_picture_t *pic,
pic->widths[plane] * bytes_per_sample);
}
+switch (pic->type) {
+case DAVS2_PIC_I:
+frame->pict_type = AV_PICTURE_TYPE_I;
+break;
+case DAVS2_PIC_P:
+frame->pict_type = AV_PICTURE_TYPE_P;
+break;
+case DAVS2_PIC_B:
+case DAVS2_PIC_F:
+frame->pict_type = AV_PICTURE_TYPE_B;
+break;
+default:
+frame->pict_type = AV_PICTURE_TYPE_NONE;
+}
+
frame->width = cad->headerset.width;
frame->height= cad->headerset.height;
frame->pts = cad->out_frame.pts;
-frame->pict_type = pic->type;
frame->format= avctx->pix_fmt;
+frame->key_frame = pic->type == DAVS2_PIC_I ? 1 : 0;
return 1;
}
--
2.7.4
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
