| Message ID | 20230721133746.33335-1-jdek@itanimul.li |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/7] lavu: add ecinfo sidedata | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Fri, Jul 21, 2023 at 9:38 AM J. Dekker <jdek@itanimul.li> wrote: > > Add sidedata for passing basic Error Concealment information helpful to > a renderer or end user attempting to filter or conceal video decoding > errors and artifacts. > > Co-Authored-By: Thomas Guillem <thomas@gllm.fr> > Signed-off-by: J. Dekker <jdek@itanimul.li> > --- > libavutil/Makefile | 2 ++ > libavutil/ec.c | 42 +++++++++++++++++++++++++++++ > libavutil/ec.h | 66 +++++++++++++++++++++++++++++++++++++++++++++ > libavutil/frame.c | 1 + > libavutil/frame.h | 6 +++++ > libavutil/version.h | 2 +- > 6 files changed, 118 insertions(+), 1 deletion(-) > create mode 100644 libavutil/ec.c > create mode 100644 libavutil/ec.h > > diff --git a/libavutil/Makefile b/libavutil/Makefile > index bd9c6f9e32..81b6b1fb8a 100644 > --- a/libavutil/Makefile > +++ b/libavutil/Makefile > @@ -28,6 +28,7 @@ HEADERS = adler32.h \ > display.h \ > dovi_meta.h \ > downmix_info.h \ > + ec.h \ > encryption_info.h \ > error.h \ > eval.h \ > @@ -124,6 +125,7 @@ OBJS = adler32.o \ > dovi_meta.o \ > downmix_info.o \ > encryption_info.o \ > + ec.o \ > error.o \ > eval.o \ > fifo.o \ > diff --git a/libavutil/ec.c b/libavutil/ec.c > new file mode 100644 > index 0000000000..762accd0a6 > --- /dev/null > +++ b/libavutil/ec.c > @@ -0,0 +1,42 @@ > +/* > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#include "config.h" > +#include "ec.h" > + > +AVECInfo *av_eci_create_side_data(AVFrame *f) > +{ > + AVBufferRef *buf = NULL; > + AVECInfo *eci = av_mallocz(sizeof(AVECInfo)); > + > + if (!eci) > + return NULL; > + > + buf = av_buffer_create((uint8_t *)eci, sizeof(AVECInfo), NULL, NULL, 0); > + if (!buf) { > + av_freep(&eci); > + return NULL; > + } > + > + if (!av_frame_new_side_data_from_buf(f, AV_FRAME_DATA_EC_INFO, buf)) { > + av_buffer_unref(&buf); > + return NULL; > + } > + > + return eci; > +} > diff --git a/libavutil/ec.h b/libavutil/ec.h > new file mode 100644 > index 0000000000..439fe876a2 > --- /dev/null > +++ b/libavutil/ec.h > @@ -0,0 +1,66 @@ > +/* > + * This file is part of FFmpeg. > + * > + * FFmpeg is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * FFmpeg is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with FFmpeg; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > + */ > + > +#ifndef AVUTIL_EC_H > +#define AVUTIL_EC_H > + > +#include <stdbool.h> > +#include <stddef.h> > +#include <stdint.h> > + > +#include "libavutil/mem.h" > +#include "libavutil/frame.h" > + > +/** > + * Error Concealment information helpful to a renderer or end user > + * attempting to filter or conceal video decoding errors and artifacts. > + */ > +typedef struct AVECInfo { > + /** > + * Integer estimating how many pixels of the video frame had decoding > + * errors. > + */ > + uint64_t error; > + /** > + * Integer estimating how many pixels of the video frame decoded > + * without error. > + */ > + uint64_t ok; > + /** > + * Integer estimating how many pixels of the video frame's reference > + * frames had decoding errors. > + */ > + uint64_t ref_error; > + /** > + * Integer estimating how many pixels of the video frame's reference > + * frames decoded without error. > + */ > + uint64_t ref_ok; > +} AVECInfo; > + > +static inline void av_eci_reset(AVECInfo *info) > +{ > + info->error = info->ok = info->ref_error = info->ref_ok = 0; > +} > + > +/** > + * Creates AVECInfo in a specified frame as side data. > + */ > +AVECInfo *av_eci_create_side_data(AVFrame *f); > + > +#endif /* AVUTIL_EC_H */ > diff --git a/libavutil/frame.c b/libavutil/frame.c > index b6cee2d886..30a838bf22 100644 > --- a/libavutil/frame.c > +++ b/libavutil/frame.c > @@ -960,6 +960,7 @@ const char *av_frame_side_data_name(enum AVFrameSideDataType type) > case AV_FRAME_DATA_DOVI_RPU_BUFFER: return "Dolby Vision RPU Data"; > case AV_FRAME_DATA_DOVI_METADATA: return "Dolby Vision Metadata"; > case AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT: return "Ambient viewing environment"; > + case AV_FRAME_DATA_EC_INFO: return "Error Concealment Information"; > } > return NULL; > } > diff --git a/libavutil/frame.h b/libavutil/frame.h > index a491315f25..08c6b96d29 100644 > --- a/libavutil/frame.h > +++ b/libavutil/frame.h > @@ -214,6 +214,12 @@ enum AVFrameSideDataType { > * Ambient viewing environment metadata, as defined by H.274. > */ > AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT, > + /** > + * Error Concealment Information helpful to a renderer attempting to filter > + * or conceal video decoding errors and artifacts. See AVECInfo defined in > + * libavutil/error.h. > + */ > + AV_FRAME_DATA_EC_INFO, > }; > > enum AVActiveFormatDescription { > diff --git a/libavutil/version.h b/libavutil/version.h > index 24af520e08..9e798b0e3f 100644 > --- a/libavutil/version.h > +++ b/libavutil/version.h > @@ -79,7 +79,7 @@ > */ > > #define LIBAVUTIL_VERSION_MAJOR 58 > -#define LIBAVUTIL_VERSION_MINOR 14 > +#define LIBAVUTIL_VERSION_MINOR 15 > #define LIBAVUTIL_VERSION_MICRO 100 > > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \ > -- > 2.41.0 > > _______________________________________________ > 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". Hi J, I appreciate the value of stats so I can tell that the stream had errors, but how is this side data "helpful to a renderer attempting to filter or conceal video decoding errors and artifacts" if there is no information relating to the region of the picture where the errors are present? Is the assumption that an application will simply have some threshold at which it decides to duplicate the previous frame rather than showing the current one? Thanks, Devin
On Fri, Jul 21, 2023 at 03:37:40PM +0200, J. Dekker wrote: > Add sidedata for passing basic Error Concealment information helpful to > a renderer or end user attempting to filter or conceal video decoding > errors and artifacts. The exported information seems too basic to perform error concealment with it. Also we have error_concealment code that does much better than what could be done from outside easily, that should be used. The commit message leaves a lot of "why", "for what", "how" questions Its not clear to me what this patchset is really trying to achieve thx [...]
Jul 21, 2023, 15:38 by jdek@itanimul.li: > Add sidedata for passing basic Error Concealment information helpful to > a renderer or end user attempting to filter or conceal video decoding > errors and artifacts. > > Co-Authored-By: Thomas Guillem <thomas@gllm.fr> > Signed-off-by: J. Dekker <jdek@itanimul.li> > --- > libavutil/Makefile | 2 ++ > libavutil/ec.c | 42 +++++++++++++++++++++++++++++ > libavutil/ec.h | 66 +++++++++++++++++++++++++++++++++++++++++++++ > libavutil/frame.c | 1 + > libavutil/frame.h | 6 +++++ > libavutil/version.h | 2 +- > 6 files changed, 118 insertions(+), 1 deletion(-) > create mode 100644 libavutil/ec.c > create mode 100644 libavutil/ec.h > Maybe using the same data structures as the new proposed hint video hint data (currently only used for damage) would save needing another side data type?
Hi Devin, Devin Heitmueller <devin.heitmueller@ltnglobal.com> writes: > On Fri, Jul 21, 2023 at 9:38 AM J. Dekker <jdek@itanimul.li> wrote: > > I appreciate the value of stats so I can tell that the stream had > errors, but how is this side data "helpful to a renderer attempting to > filter or conceal video decoding errors and artifacts" if there is no > information relating to the region of the picture where the errors are > present? Is the assumption that an application will simply have some > threshold at which it decides to duplicate the previous frame rather > than showing the current one? Yes, this is primarily the usecase here initially. As some point a renderer would rather duplicate previous frame than rely on reconstruction. This could be done within EC itself but letting the decoder make this choice rather than exporting the information to a renderer and doing it there seems incorrect to me. It is the intention to expand the metadata to provide information about specific regions in the frame in the future but mainly just providing the framework for providing this sort of information in the first place at the moment.
On Tue, Jul 25, 2023 at 02:05:43PM +0200, J. Dekker wrote: > > Hi Devin, > > Devin Heitmueller <devin.heitmueller@ltnglobal.com> writes: > > On Fri, Jul 21, 2023 at 9:38 AM J. Dekker <jdek@itanimul.li> wrote: > > > > I appreciate the value of stats so I can tell that the stream had > > errors, but how is this side data "helpful to a renderer attempting to > > filter or conceal video decoding errors and artifacts" if there is no > > information relating to the region of the picture where the errors are > > present? Is the assumption that an application will simply have some > > threshold at which it decides to duplicate the previous frame rather > > than showing the current one? > > Yes, this is primarily the usecase here initially. As some point a > renderer would rather duplicate previous frame than rely on > reconstruction. This could be done within EC itself but letting the > decoder make this choice rather than exporting the information to a > renderer and doing it there seems incorrect to me. Error concealment is not merely a choice of copying or not Some areas may be damaged, surrounding areas may be undamaged an example could be a row of macroblocks / a slice To perform error concealment properly you need to know the motion vectors of the surrounding slices and their macroblock types for example if all the surrounding macroblocks have a motion vector pointing left by 7 pixels the lost slice likely needs to be using the same. This requires also knowledge of the reference frames the decoder used and ability to apply similar motion compensation to know what "-7" means If there are different motion vecstors used in surrounding macroblocks it becomes even more complex Also with some decoders you can loose the texture of a slice but have undamaged motion vectors. Or you may have intra slices with DC undamaged but AC lost for every macroblock And if its all intra blocks, some inpainting based on spatial surroundings may be better than using the previosu frames. This is all implemented in our error concealment code. Trying to do this outside is a mistake. Not only is it hard due to the deep connection to decoder state it is also something every user of libavcodec needs. The code belongs in a place every libavcodec user has access to and can reuse it from. Thx [...]
diff --git a/libavutil/Makefile b/libavutil/Makefile index bd9c6f9e32..81b6b1fb8a 100644 --- a/libavutil/Makefile +++ b/libavutil/Makefile @@ -28,6 +28,7 @@ HEADERS = adler32.h \ display.h \ dovi_meta.h \ downmix_info.h \ + ec.h \ encryption_info.h \ error.h \ eval.h \ @@ -124,6 +125,7 @@ OBJS = adler32.o \ dovi_meta.o \ downmix_info.o \ encryption_info.o \ + ec.o \ error.o \ eval.o \ fifo.o \ diff --git a/libavutil/ec.c b/libavutil/ec.c new file mode 100644 index 0000000000..762accd0a6 --- /dev/null +++ b/libavutil/ec.c @@ -0,0 +1,42 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include "config.h" +#include "ec.h" + +AVECInfo *av_eci_create_side_data(AVFrame *f) +{ + AVBufferRef *buf = NULL; + AVECInfo *eci = av_mallocz(sizeof(AVECInfo)); + + if (!eci) + return NULL; + + buf = av_buffer_create((uint8_t *)eci, sizeof(AVECInfo), NULL, NULL, 0); + if (!buf) { + av_freep(&eci); + return NULL; + } + + if (!av_frame_new_side_data_from_buf(f, AV_FRAME_DATA_EC_INFO, buf)) { + av_buffer_unref(&buf); + return NULL; + } + + return eci; +} diff --git a/libavutil/ec.h b/libavutil/ec.h new file mode 100644 index 0000000000..439fe876a2 --- /dev/null +++ b/libavutil/ec.h @@ -0,0 +1,66 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef AVUTIL_EC_H +#define AVUTIL_EC_H + +#include <stdbool.h> +#include <stddef.h> +#include <stdint.h> + +#include "libavutil/mem.h" +#include "libavutil/frame.h" + +/** + * Error Concealment information helpful to a renderer or end user + * attempting to filter or conceal video decoding errors and artifacts. + */ +typedef struct AVECInfo { + /** + * Integer estimating how many pixels of the video frame had decoding + * errors. + */ + uint64_t error; + /** + * Integer estimating how many pixels of the video frame decoded + * without error. + */ + uint64_t ok; + /** + * Integer estimating how many pixels of the video frame's reference + * frames had decoding errors. + */ + uint64_t ref_error; + /** + * Integer estimating how many pixels of the video frame's reference + * frames decoded without error. + */ + uint64_t ref_ok; +} AVECInfo; + +static inline void av_eci_reset(AVECInfo *info) +{ + info->error = info->ok = info->ref_error = info->ref_ok = 0; +} + +/** + * Creates AVECInfo in a specified frame as side data. + */ +AVECInfo *av_eci_create_side_data(AVFrame *f); + +#endif /* AVUTIL_EC_H */ diff --git a/libavutil/frame.c b/libavutil/frame.c index b6cee2d886..30a838bf22 100644 --- a/libavutil/frame.c +++ b/libavutil/frame.c @@ -960,6 +960,7 @@ const char *av_frame_side_data_name(enum AVFrameSideDataType type) case AV_FRAME_DATA_DOVI_RPU_BUFFER: return "Dolby Vision RPU Data"; case AV_FRAME_DATA_DOVI_METADATA: return "Dolby Vision Metadata"; case AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT: return "Ambient viewing environment"; + case AV_FRAME_DATA_EC_INFO: return "Error Concealment Information"; } return NULL; } diff --git a/libavutil/frame.h b/libavutil/frame.h index a491315f25..08c6b96d29 100644 --- a/libavutil/frame.h +++ b/libavutil/frame.h @@ -214,6 +214,12 @@ enum AVFrameSideDataType { * Ambient viewing environment metadata, as defined by H.274. */ AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT, + /** + * Error Concealment Information helpful to a renderer attempting to filter + * or conceal video decoding errors and artifacts. See AVECInfo defined in + * libavutil/error.h. + */ + AV_FRAME_DATA_EC_INFO, }; enum AVActiveFormatDescription { diff --git a/libavutil/version.h b/libavutil/version.h index 24af520e08..9e798b0e3f 100644 --- a/libavutil/version.h +++ b/libavutil/version.h @@ -79,7 +79,7 @@ */ #define LIBAVUTIL_VERSION_MAJOR 58 -#define LIBAVUTIL_VERSION_MINOR 14 +#define LIBAVUTIL_VERSION_MINOR 15 #define LIBAVUTIL_VERSION_MICRO 100 #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
Add sidedata for passing basic Error Concealment information helpful to a renderer or end user attempting to filter or conceal video decoding errors and artifacts. Co-Authored-By: Thomas Guillem <thomas@gllm.fr> Signed-off-by: J. Dekker <jdek@itanimul.li> --- libavutil/Makefile | 2 ++ libavutil/ec.c | 42 +++++++++++++++++++++++++++++ libavutil/ec.h | 66 +++++++++++++++++++++++++++++++++++++++++++++ libavutil/frame.c | 1 + libavutil/frame.h | 6 +++++ libavutil/version.h | 2 +- 6 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 libavutil/ec.c create mode 100644 libavutil/ec.h