Message ID | 20210224102542.31705-1-anton@khirnov.net |
---|---|
State | Accepted |
Commit | 9e4225cf7f26b57e0054470127bcc032b6d29742 |
Headers | show |
Series | [FFmpeg-devel] Handle AVID MJPEG streams directly in the MJPEG decoder. | 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 Wed, Feb 24, 2021 at 11:25:42AM +0100, Anton Khirnov wrote: > AVID streams - currently handled by the AVRN decoder - can be (depending > on extradata contents) either MJPEG or raw video. To decode the MJPEG > variant, the AVRN decoder currently instantiates a MJPEG decoder > internally and forwards decoded frames to the caller (possibly after > cropping them). > > This is suboptimal, because the AVRN decoder does not forward all the > features of the internal MJPEG decoder, such as direct rendering. > Handling such forwarding in a full and generic manner would be quite > hard, so it is simpler to just handle those streams in the MJPEG decoder > directly. > > The AVRN decoder, which now handles only the raw streams, can now be > marked as supporting direct rendering. > > This also removes the last remaining internal use of the obsolete > decoding API. > --- > And now without the missing isom tag change. Thanks to Andreas for noticing. > --- > configure | 1 - > libavcodec/avrndec.c | 71 +---------------------------------------- > libavcodec/mjpegdec.c | 11 +++++++ > libavcodec/version.h | 2 +- > libavformat/avidec.c | 6 ++++ > libavformat/isom_tags.c | 4 +-- > tests/fate/video.mak | 2 +- > 7 files changed, 22 insertions(+), 75 deletions(-) breaks: ./ffmpeg -vlowres 2 -i ~/tickets/162/avid.avi -vframes 3 avrn.avi The outputed image is several times bigger and looks as if it encodes random bits od memory thx [...]
On 2/27/2021 1:33 PM, Michael Niedermayer wrote: > On Wed, Feb 24, 2021 at 11:25:42AM +0100, Anton Khirnov wrote: >> AVID streams - currently handled by the AVRN decoder - can be (depending >> on extradata contents) either MJPEG or raw video. To decode the MJPEG >> variant, the AVRN decoder currently instantiates a MJPEG decoder >> internally and forwards decoded frames to the caller (possibly after >> cropping them). >> >> This is suboptimal, because the AVRN decoder does not forward all the >> features of the internal MJPEG decoder, such as direct rendering. >> Handling such forwarding in a full and generic manner would be quite >> hard, so it is simpler to just handle those streams in the MJPEG decoder >> directly. >> >> The AVRN decoder, which now handles only the raw streams, can now be >> marked as supporting direct rendering. >> >> This also removes the last remaining internal use of the obsolete >> decoding API. >> --- >> And now without the missing isom tag change. Thanks to Andreas for noticing. >> --- >> configure | 1 - >> libavcodec/avrndec.c | 71 +---------------------------------------- >> libavcodec/mjpegdec.c | 11 +++++++ >> libavcodec/version.h | 2 +- >> libavformat/avidec.c | 6 ++++ >> libavformat/isom_tags.c | 4 +-- >> tests/fate/video.mak | 2 +- >> 7 files changed, 22 insertions(+), 75 deletions(-) > > breaks: > ./ffmpeg -vlowres 2 -i ~/tickets/162/avid.avi -vframes 3 avrn.avi > > The outputed image is several times bigger and looks as if it encodes random bits od memory Can you try again with git head? I removed the max_lowres value since the avrn raw decoder doesn't support it, since it was pulled from the mjpeg version. > > thx > > [...] > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On 2/27/2021 1:37 PM, James Almer wrote: > On 2/27/2021 1:33 PM, Michael Niedermayer wrote: >> On Wed, Feb 24, 2021 at 11:25:42AM +0100, Anton Khirnov wrote: >>> AVID streams - currently handled by the AVRN decoder - can be (depending >>> on extradata contents) either MJPEG or raw video. To decode the MJPEG >>> variant, the AVRN decoder currently instantiates a MJPEG decoder >>> internally and forwards decoded frames to the caller (possibly after >>> cropping them). >>> >>> This is suboptimal, because the AVRN decoder does not forward all the >>> features of the internal MJPEG decoder, such as direct rendering. >>> Handling such forwarding in a full and generic manner would be quite >>> hard, so it is simpler to just handle those streams in the MJPEG decoder >>> directly. >>> >>> The AVRN decoder, which now handles only the raw streams, can now be >>> marked as supporting direct rendering. >>> >>> This also removes the last remaining internal use of the obsolete >>> decoding API. >>> --- >>> And now without the missing isom tag change. Thanks to Andreas for >>> noticing. >>> --- >>> configure | 1 - >>> libavcodec/avrndec.c | 71 +---------------------------------------- >>> libavcodec/mjpegdec.c | 11 +++++++ >>> libavcodec/version.h | 2 +- >>> libavformat/avidec.c | 6 ++++ >>> libavformat/isom_tags.c | 4 +-- >>> tests/fate/video.mak | 2 +- >>> 7 files changed, 22 insertions(+), 75 deletions(-) >> >> breaks: >> ./ffmpeg -vlowres 2 -i ~/tickets/162/avid.avi -vframes 3 avrn.avi >> >> The outputed image is several times bigger and looks as if it encodes >> random bits od memory > > Can you try again with git head? I removed the max_lowres value since > the avrn raw decoder doesn't support it, since it was pulled from the > mjpeg version. Ok, the above was unrelated since this sample is mjpeg, now raw. The following however fixes it for me: > diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c > index 79c7d6e6cf..8eed8eefe6 100644 > --- a/libavcodec/mjpegdec.c > +++ b/libavcodec/mjpegdec.c > @@ -2863,8 +2863,8 @@ the_end: > if ((avctx->codec_tag == MKTAG('A', 'V', 'R', 'n') || > avctx->codec_tag == MKTAG('A', 'V', 'D', 'J')) && > avctx->coded_height > s->orig_height) { > - frame->height = avctx->coded_height; > - frame->crop_top = frame->height - s->orig_height; > + frame->height = AV_CEIL_RSHIFT(avctx->coded_height, avctx->lowres); > + frame->crop_top = frame->height - AV_CEIL_RSHIFT(s->orig_height, avctx->lowres); But it may be an incomplete solution, since there's another similar check/assignment in ff_mjpeg_decode_sof() also added by 9e4225cf7f.
diff --git a/configure b/configure index 5250445325..d11942fced 100755 --- a/configure +++ b/configure @@ -2688,7 +2688,6 @@ atrac3p_decoder_select="mdct sinewin" atrac3pal_decoder_select="mdct sinewin" atrac9_decoder_select="mdct" av1_decoder_select="cbs_av1" -avrn_decoder_select="mjpeg_decoder" bink_decoder_select="blockdsp hpeldsp" binkaudio_dct_decoder_select="mdct rdft dct sinewin wma_freqs" binkaudio_rdft_decoder_select="mdct rdft sinewin wma_freqs" diff --git a/libavcodec/avrndec.c b/libavcodec/avrndec.c index 9380d86885..26cf6b752c 100644 --- a/libavcodec/avrndec.c +++ b/libavcodec/avrndec.c @@ -24,8 +24,6 @@ #include "libavutil/imgutils.h" typedef struct { - AVCodecContext *mjpeg_avctx; - int is_mjpeg; int interlace; int tff; } AVRnContext; @@ -35,42 +33,6 @@ static av_cold int init(AVCodecContext *avctx) AVRnContext *a = avctx->priv_data; int ret; - // Support "Resolution 1:1" for Avid AVI Codec - a->is_mjpeg = avctx->extradata_size < 31 || memcmp(&avctx->extradata[28], "1:1", 3); - - if(!a->is_mjpeg && avctx->lowres) { - av_log(avctx, AV_LOG_ERROR, "lowres is not possible with rawvideo\n"); - return AVERROR(EINVAL); - } - - if(a->is_mjpeg) { - const AVCodec *codec = avcodec_find_decoder(AV_CODEC_ID_MJPEG); - AVDictionary *thread_opt = NULL; - if (!codec) { - av_log(avctx, AV_LOG_ERROR, "MJPEG codec not found\n"); - return AVERROR_DECODER_NOT_FOUND; - } - - a->mjpeg_avctx = avcodec_alloc_context3(codec); - if (!a->mjpeg_avctx) - return AVERROR(ENOMEM); - - av_dict_set(&thread_opt, "threads", "1", 0); // Is this needed ? - a->mjpeg_avctx->refcounted_frames = 1; - a->mjpeg_avctx->flags = avctx->flags; - a->mjpeg_avctx->idct_algo = avctx->idct_algo; - a->mjpeg_avctx->lowres = avctx->lowres; - a->mjpeg_avctx->width = avctx->width; - a->mjpeg_avctx->height = avctx->height; - - if ((ret = avcodec_open2(a->mjpeg_avctx, codec, &thread_opt)) < 0) { - av_log(avctx, AV_LOG_ERROR, "MJPEG codec failed to open\n"); - } - av_dict_free(&thread_opt); - - return ret; - } - if ((ret = av_image_check_size(avctx->width, avctx->height, 0, avctx)) < 0) return ret; @@ -87,15 +49,6 @@ static av_cold int init(AVCodecContext *avctx) return 0; } -static av_cold int end(AVCodecContext *avctx) -{ - AVRnContext *a = avctx->priv_data; - - avcodec_free_context(&a->mjpeg_avctx); - - return 0; -} - static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, AVPacket *avpkt) { @@ -105,28 +58,6 @@ static int decode_frame(AVCodecContext *avctx, void *data, int buf_size = avpkt->size; int y, ret, true_height; - if(a->is_mjpeg) { - ret = avcodec_decode_video2(a->mjpeg_avctx, data, got_frame, avpkt); - - if (ret >= 0 && *got_frame && avctx->width <= p->width && avctx->height <= p->height) { - int shift = p->height - avctx->height; - int subsample_h, subsample_v; - - av_pix_fmt_get_chroma_sub_sample(p->format, &subsample_h, &subsample_v); - - p->data[0] += p->linesize[0] * shift; - if (p->data[2]) { - p->data[1] += p->linesize[1] * (shift>>subsample_v); - p->data[2] += p->linesize[2] * (shift>>subsample_v); - } - - p->width = avctx->width; - p->height = avctx->height; - } - avctx->pix_fmt = a->mjpeg_avctx->pix_fmt; - return ret; - } - true_height = buf_size / (2*avctx->width); if(buf_size < 2*avctx->width * avctx->height) { @@ -165,8 +96,8 @@ AVCodec ff_avrn_decoder = { .id = AV_CODEC_ID_AVRN, .priv_data_size = sizeof(AVRnContext), .init = init, - .close = end, .decode = decode_frame, .max_lowres = 3, + .capabilities = AV_CODEC_CAP_DR1, .caps_internal = FF_CODEC_CAP_INIT_THREADSAFE | FF_CODEC_CAP_INIT_CLEANUP, }; diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c index 256393a68f..79c7d6e6cf 100644 --- a/libavcodec/mjpegdec.c +++ b/libavcodec/mjpegdec.c @@ -450,6 +450,11 @@ int ff_mjpeg_decode_sof(MJpegDecodeContext *s) size_change = 0; } + if ((s->avctx->codec_tag == MKTAG('A', 'V', 'R', 'n') || + s->avctx->codec_tag == MKTAG('A', 'V', 'D', 'J')) && + s->orig_height < s->avctx->height) + s->avctx->height = s->orig_height; + if (s->avctx->codec_id == AV_CODEC_ID_SMVJPEG) { s->avctx->height = s->avctx->coded_height / s->smv_frames_per_jpeg; if (s->avctx->height <= 0) @@ -2855,6 +2860,12 @@ the_end: return ret; } } + if ((avctx->codec_tag == MKTAG('A', 'V', 'R', 'n') || + avctx->codec_tag == MKTAG('A', 'V', 'D', 'J')) && + avctx->coded_height > s->orig_height) { + frame->height = avctx->coded_height; + frame->crop_top = frame->height - s->orig_height; + } ret = 0; diff --git a/libavcodec/version.h b/libavcodec/version.h index a0c9241fe6..e5a5ec8abc 100644 --- a/libavcodec/version.h +++ b/libavcodec/version.h @@ -29,7 +29,7 @@ #define LIBAVCODEC_VERSION_MAJOR 58 #define LIBAVCODEC_VERSION_MINOR 125 -#define LIBAVCODEC_VERSION_MICRO 100 +#define LIBAVCODEC_VERSION_MICRO 101 #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \ LIBAVCODEC_VERSION_MINOR, \ diff --git a/libavformat/avidec.c b/libavformat/avidec.c index 547eb63b1b..5ea6160eef 100644 --- a/libavformat/avidec.c +++ b/libavformat/avidec.c @@ -838,6 +838,12 @@ static int avi_read_header(AVFormatContext *s) st->codecpar->codec_tag == MKTAG('H', '2', '6', '5')) st->need_parsing = AVSTREAM_PARSE_FULL; + if (st->codecpar->codec_id == AV_CODEC_ID_AVRN && + st->codecpar->codec_tag == MKTAG('A', 'V', 'R', 'n') && + (st->codecpar->extradata_size < 31 || + memcmp(&st->codecpar->extradata[28], "1:1", 3))) + st->codecpar->codec_id = AV_CODEC_ID_MJPEG; + if (st->codecpar->codec_tag == 0 && st->codecpar->height > 0 && st->codecpar->extradata_size < 1U << 30) { st->codecpar->extradata_size += 9; diff --git a/libavformat/isom_tags.c b/libavformat/isom_tags.c index 75c3ee2b32..1666b9d4a5 100644 --- a/libavformat/isom_tags.c +++ b/libavformat/isom_tags.c @@ -69,8 +69,8 @@ const AVCodecTag ff_codec_movvideo_tags[] = { { AV_CODEC_ID_MJPEG, MKTAG('j', 'p', 'e', 'g') }, /* PhotoJPEG */ { AV_CODEC_ID_MJPEG, MKTAG('m', 'j', 'p', 'a') }, /* Motion-JPEG (format A) */ - { AV_CODEC_ID_AVRN , MKTAG('A', 'V', 'D', 'J') }, /* MJPEG with alpha-channel (AVID JFIF meridien compressed) */ -/* { AV_CODEC_ID_MJPEG, MKTAG('A', 'V', 'R', 'n') }, *//* MJPEG with alpha-channel (AVID ABVB/Truevision NuVista) */ + { AV_CODEC_ID_MJPEG, MKTAG('A', 'V', 'D', 'J') }, /* MJPEG with alpha-channel (AVID JFIF meridien compressed) */ + { AV_CODEC_ID_MJPEG, MKTAG('A', 'V', 'R', 'n') }, /* MJPEG with alpha-channel (AVID ABVB/Truevision NuVista) */ { AV_CODEC_ID_MJPEG, MKTAG('d', 'm', 'b', '1') }, /* Motion JPEG OpenDML */ { AV_CODEC_ID_MJPEGB, MKTAG('m', 'j', 'p', 'b') }, /* Motion-JPEG (format B) */ diff --git a/tests/fate/video.mak b/tests/fate/video.mak index a5f3107c38..0984b929cb 100644 --- a/tests/fate/video.mak +++ b/tests/fate/video.mak @@ -46,7 +46,7 @@ fate-auravision-v2: CMD = framecrc -i $(TARGET_SAMPLES)/auravision/salma-hayek-i FATE_VIDEO-$(call DEMDEC, AVI, AVRN) += fate-avid-interlaced fate-avid-interlaced: CMD = framecrc -i $(TARGET_SAMPLES)/avid/avid_ntsc_interlaced.avi -FATE_VIDEO-$(call DEMDEC, MOV, AVRN) += fate-avid-meridian +FATE_VIDEO-$(call DEMDEC, MOV, MJPEG) += fate-avid-meridian fate-avid-meridian: CMD = framecrc -i $(TARGET_SAMPLES)/avid/avidmeridianntsc.mov FATE_VIDEO-$(call DEMDEC, BETHSOFTVID, BETHSOFTVID) += fate-bethsoft-vid