diff mbox series

[FFmpeg-devel,1/1] Added avs2 tags for MKV

Message ID BYAPR03MB4536A08EDB10462539DF4C08B85C0@BYAPR03MB4536.namprd03.prod.outlook.com
State New
Headers show
Series [FFmpeg-devel,1/1] Added avs2 tags for MKV | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Ze Yuan Aug. 18, 2020, 10 p.m. UTC
From d4ae49d8fabdb70435c92457c153a869dc55a980 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E9=83=91=E5=A4=A9=E6=B3=A2?= <naturalwalker@hotmail.com>
Date: Wed, 19 Aug 2020 05:20:18 +0800
Subject: [PATCH] Added avs2 tags for MKV container.

Before the patch, an avs2 encoded MKV video results in dropped frames in asv2 enabled video players.

Here are the asv2/avs3 enabled video players:
VLC 3: https://github.com/xatabhk/vlc-3.0-avs2-avs3/releases/tag/v3.0-avs2-avs3-200812
Mpc-hc: https://gitee.com/zhengtianbo/cavs-avs2-avs3_decoder_added_to_mpc_hc/releases

---
libavformat/isom.c     | 3 ++-
libavformat/matroska.c | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)

Comments

Andreas Rheinhardt Aug. 18, 2020, 10:18 p.m. UTC | #1
Ze Yuan:
> From d4ae49d8fabdb70435c92457c153a869dc55a980 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?=E9=83=91=E5=A4=A9=E6=B3=A2?= <naturalwalker@hotmail.com>
> Date: Wed, 19 Aug 2020 05:20:18 +0800
> Subject: [PATCH] Added avs2 tags for MKV container.
> 
> Before the patch, an avs2 encoded MKV video results in dropped frames in asv2 enabled video players.
> 
> Here are the asv2/avs3 enabled video players:
> VLC 3: https://github.com/xatabhk/vlc-3.0-avs2-avs3/releases/tag/v3.0-avs2-avs3-200812
> Mpc-hc: https://gitee.com/zhengtianbo/cavs-avs2-avs3_decoder_added_to_mpc_hc/releases

The ordinary versions of these players don't use libavformat's Matroska
demuxer at all, so I presume that your patch intends to fix the problem
by changing how the files are muxed. This information should actually be
directly included in the commit message so that one doesn't have to guess.

> 
> ---
> libavformat/isom.c     | 3 ++-
> libavformat/matroska.c | 1 +
> 2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/isom.c b/libavformat/isom.c
> index 209bbac5d1..080a457a4e 100644
> --- a/libavformat/isom.c
> +++ b/libavformat/isom.c
> @@ -255,7 +255,8 @@ const AVCodecTag ff_codec_movvideo_tags[] = {
>      { AV_CODEC_ID_PNG,   MKTAG('M', 'N', 'G', ' ') },
> 
>      { AV_CODEC_ID_VC1, MKTAG('v', 'c', '-', '1') }, /* SMPTE RP 2025 */
> -    { AV_CODEC_ID_CAVS, MKTAG('a', 'v', 's', '2') },
> +    { AV_CODEC_ID_CAVS, MKTAG('c', 'a', 'v', 's') },
> +    { AV_CODEC_ID_AVS2, MKTAG('a', 'v', 's', '2') },

By changing ff_codec_movvideo_tags you are changing more than just
Matroska in contradiction to what the commit message states.

> 
>      { AV_CODEC_ID_DIRAC,     MKTAG('d', 'r', 'a', 'c') },
>      { AV_CODEC_ID_DNXHD,     MKTAG('A', 'V', 'd', 'n') }, /* AVID DNxHD */
> diff --git a/libavformat/matroska.c b/libavformat/matroska.c
> index 7c56aba403..2ce60c6277 100644
> --- a/libavformat/matroska.c
> +++ b/libavformat/matroska.c
> @@ -78,6 +78,7 @@ const CodecTags ff_mkv_codec_tags[]={
>      {"S_HDMV/TEXTST"    , AV_CODEC_ID_HDMV_TEXT_SUBTITLE},
> 
>      {"V_AV1"            , AV_CODEC_ID_AV1},
> +    {"V_AVS2"           , AV_CODEC_ID_AVS2},

Matroska does not support AVS2 natively at all atm. If you want to
change that, you should open an issue/pull request at [1]. And you
should refrain from already creating such files (or use a CodecID like
"V_AVS2/EXPERIMENTAL"). As long as there is no official support for AVS2
in Matroska, there won't be support for it in FFmpeg.

- Andreas

[1]: https://github.com/cellar-wg/matroska-specification/
diff mbox series

Patch

diff --git a/libavformat/isom.c b/libavformat/isom.c
index 209bbac5d1..080a457a4e 100644
--- a/libavformat/isom.c
+++ b/libavformat/isom.c
@@ -255,7 +255,8 @@  const AVCodecTag ff_codec_movvideo_tags[] = {
     { AV_CODEC_ID_PNG,   MKTAG('M', 'N', 'G', ' ') },

     { AV_CODEC_ID_VC1, MKTAG('v', 'c', '-', '1') }, /* SMPTE RP 2025 */
-    { AV_CODEC_ID_CAVS, MKTAG('a', 'v', 's', '2') },
+    { AV_CODEC_ID_CAVS, MKTAG('c', 'a', 'v', 's') },
+    { AV_CODEC_ID_AVS2, MKTAG('a', 'v', 's', '2') },

     { AV_CODEC_ID_DIRAC,     MKTAG('d', 'r', 'a', 'c') },
     { AV_CODEC_ID_DNXHD,     MKTAG('A', 'V', 'd', 'n') }, /* AVID DNxHD */
diff --git a/libavformat/matroska.c b/libavformat/matroska.c
index 7c56aba403..2ce60c6277 100644
--- a/libavformat/matroska.c
+++ b/libavformat/matroska.c
@@ -78,6 +78,7 @@  const CodecTags ff_mkv_codec_tags[]={
     {"S_HDMV/TEXTST"    , AV_CODEC_ID_HDMV_TEXT_SUBTITLE},

     {"V_AV1"            , AV_CODEC_ID_AV1},
+    {"V_AVS2"           , AV_CODEC_ID_AVS2},
     {"V_DIRAC"          , AV_CODEC_ID_DIRAC},
     {"V_FFV1"           , AV_CODEC_ID_FFV1},
     {"V_MJPEG"          , AV_CODEC_ID_MJPEG},