| Message ID | 1540981431-6995-3-git-send-email-hwrenx@126.com |
|---|---|
| State | Accepted |
| Headers | show |
2018-10-31 11:23 GMT+01:00, hwren <hwrenx@126.com>: > Signed-off-by: hwren <hwrenx@126.com> > --- > 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
On 31/10/18 10:23, hwren wrote: > Signed-off-by: hwren <hwrenx@126.com> > --- > 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
2018-10-31 23:15 GMT+01:00, Mark Thompson <sw@jkqxz.net>: > 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
At 2018-11-01 08:40:50, "Carl Eugen Hoyos" <ceffmpeg@gmail.com> wrote: >2018-10-31 23:15 GMT+01:00, Mark Thompson <sw@jkqxz.net>: >> 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
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; }
Signed-off-by: hwren <hwrenx@126.com> --- libavcodec/libdavs2.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)