| Message ID | CAGTf1Mkv2bFyTSNJm4DzOEw1KMwWB2AgNvQ-mhamu3K4ht4jbA@mail.gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
A gentle ping... On Fri, Oct 12, 2018 at 9:14 AM Valery Kot <valery.kot@gmail.com> wrote: > > When using libx264 (GPL) encoder, sample aspect ratio gets stored on > both container and frame levels. For libopenh264 (LGPL) encoder, > aspect ratio on codec/frame level currently is ignored, which results > in weird display aspect ratio for non-square pixels on some players. > > Proposed patch fixes that, if FFmpeg being build against libopenh264 > 1.7 or newer.
Another ping. I understand that there is still (or anymore) no active maintainer for this module. Could any of the developers please take a look at this patch and push it? I really need it in FFmpeg repo! On Thu, Oct 18, 2018 at 10:46 AM Valery Kot <valery.kot@gmail.com> wrote: > > A gentle ping...
Yet another ping... Apparently there are no acrive maintainers and no devs (other than me) using libopenh264. What can be done to have this patch accepted? On Wed, Oct 24, 2018 at 8:57 AM Valery Kot <valery.kot@gmail.com> wrote: > > Another ping. > On Thu, Oct 18, 2018 at 10:46 AM Valery Kot <valery.kot@gmail.com> wrote: > > > > A gentle ping...
2018-10-31 9:12 GMT+01:00, Valery Kot <valery.kot@gmail.com>: > Yet another ping... > Apparently there are no acrive maintainers and no devs (other than me) > using libopenh264. What can be done to have this patch accepted? One possibility is that you send a patch that adds yourself as maintainer. Carl Eugen
> > Apparently there are no acrive maintainers and no devs (other than me) > > using libopenh264. What can be done to have this patch accepted? > > One possibility is that you send a patch that adds yourself > as maintainer. > > Carl Eugen Patch for MAINTAINERS list submitted, as you suggested.
On 12/10/18 08:14, Valery Kot wrote: > When using libx264 (GPL) encoder, sample aspect ratio gets stored on > both container and frame levels. For libopenh264 (LGPL) encoder, > aspect ratio on codec/frame level currently is ignored, which results > in weird display aspect ratio for non-square pixels on some players. > > Proposed patch fixes that, if FFmpeg being build against libopenh264 > 1.7 or newer. > > From 2d22329e01b892576b856806c93d484c304f11d8 Mon Sep 17 00:00:00 2001 > From: vkot <valery.kot@4cinsights.com> > Date: Fri, 12 Oct 2018 09:02:59 +0200 > Subject: [PATCH] Handle sample_aspect_ratio in libopenh264 encoder > > --- > libavcodec/libopenh264enc.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c > index 83c3f0ce20..c630dff33c 100644 > --- a/libavcodec/libopenh264enc.c > +++ b/libavcodec/libopenh264enc.c > @@ -164,6 +164,32 @@ FF_ENABLE_DEPRECATION_WARNINGS > param.sSpatialLayers[0].iSpatialBitrate = param.iTargetBitrate; > param.sSpatialLayers[0].iMaxSpatialBitrate = param.iMaxBitrate; > > +#if OPENH264_VER_AT_LEAST(1, 7) > + if(avctx->sample_aspect_ratio.num == 0 || avctx->sample_aspect_ratio.den == 0) > + param.sSpatialLayers[0].bAspectRatioPresent = false; > + else { Reduce the fraction to have numerator/denominator fitting in 16 bits before doing the following - if libopenh264 doesn't handle the aspect_ratio_idc values itself then I doubt it does the reduction needed for the extended values. > + param.sSpatialLayers[0].bAspectRatioPresent = true; > + if (!av_cmp_q(av_make_q( 1, 1), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_1x1; > + else if (!av_cmp_q(av_make_q(12, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_12x11; > + else if (!av_cmp_q(av_make_q(10, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_10x11; > + else if (!av_cmp_q(av_make_q(16, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_16x11; > + else if (!av_cmp_q(av_make_q(40, 33), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_40x33; > + else if (!av_cmp_q(av_make_q(24, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_24x11; > + else if (!av_cmp_q(av_make_q(20, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_20x11; > + else if (!av_cmp_q(av_make_q(32, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_32x11; > + else if (!av_cmp_q(av_make_q(80, 33), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_80x33; > + else if (!av_cmp_q(av_make_q(18, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_18x11; > + else if (!av_cmp_q(av_make_q(15, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_15x11; > + else if (!av_cmp_q(av_make_q(64, 33), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_64x33; > + else if (!av_cmp_q(av_make_q(160,99), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_160x99; I think this would look nicer (and generate less code) as a table + loop rather than an if-ladder making each fraction structure inline? E.g. something like <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/h264_metadata_bsf.c;h=bf37528234c7ab8bac56f773ad99f5a67f79db29;hb=HEAD#l95>. > + else { > + param.sSpatialLayers[0].eAspectRatio = ASP_EXT_SAR; > + param.sSpatialLayers[0].sAspectRatioExtWidth = avctx->sample_aspect_ratio.num; > + param.sSpatialLayers[0].sAspectRatioExtHeight = avctx->sample_aspect_ratio.den; > + } > + } > +#endif > + > if ((avctx->slices > 1) && (s->max_nal_size)) { > av_log(avctx, AV_LOG_ERROR, > "Invalid combination -slices %d and -max_nal_size %d.\n", > -- > 2.15.1.windows.2 > Thanks, - Mark
diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c index 83c3f0ce20..c630dff33c 100644 --- a/libavcodec/libopenh264enc.c +++ b/libavcodec/libopenh264enc.c @@ -164,6 +164,32 @@ FF_ENABLE_DEPRECATION_WARNINGS param.sSpatialLayers[0].iSpatialBitrate = param.iTargetBitrate; param.sSpatialLayers[0].iMaxSpatialBitrate = param.iMaxBitrate; +#if OPENH264_VER_AT_LEAST(1, 7) + if(avctx->sample_aspect_ratio.num == 0 || avctx->sample_aspect_ratio.den == 0) + param.sSpatialLayers[0].bAspectRatioPresent = false; + else { + param.sSpatialLayers[0].bAspectRatioPresent = true; + if (!av_cmp_q(av_make_q( 1, 1), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_1x1; + else if (!av_cmp_q(av_make_q(12, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_12x11; + else if (!av_cmp_q(av_make_q(10, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_10x11; + else if (!av_cmp_q(av_make_q(16, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_16x11; + else if (!av_cmp_q(av_make_q(40, 33), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_40x33; + else if (!av_cmp_q(av_make_q(24, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_24x11; + else if (!av_cmp_q(av_make_q(20, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_20x11; + else if (!av_cmp_q(av_make_q(32, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_32x11; + else if (!av_cmp_q(av_make_q(80, 33), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_80x33; + else if (!av_cmp_q(av_make_q(18, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_18x11; + else if (!av_cmp_q(av_make_q(15, 11), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_15x11; + else if (!av_cmp_q(av_make_q(64, 33), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_64x33; + else if (!av_cmp_q(av_make_q(160,99), avctx->sample_aspect_ratio)) param.sSpatialLayers[0].eAspectRatio = ASP_160x99; + else { + param.sSpatialLayers[0].eAspectRatio = ASP_EXT_SAR; + param.sSpatialLayers[0].sAspectRatioExtWidth = avctx->sample_aspect_ratio.num; + param.sSpatialLayers[0].sAspectRatioExtHeight = avctx->sample_aspect_ratio.den; + } + } +#endif + if ((avctx->slices > 1) && (s->max_nal_size)) { av_log(avctx, AV_LOG_ERROR, "Invalid combination -slices %d and -max_nal_size %d.\n",
When using libx264 (GPL) encoder, sample aspect ratio gets stored on both container and frame levels. For libopenh264 (LGPL) encoder, aspect ratio on codec/frame level currently is ignored, which results in weird display aspect ratio for non-square pixels on some players. Proposed patch fixes that, if FFmpeg being build against libopenh264 1.7 or newer. From 2d22329e01b892576b856806c93d484c304f11d8 Mon Sep 17 00:00:00 2001 From: vkot <valery.kot@4cinsights.com> Date: Fri, 12 Oct 2018 09:02:59 +0200 Subject: [PATCH] Handle sample_aspect_ratio in libopenh264 encoder --- libavcodec/libopenh264enc.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)