On 01/11/18 13:24, Valery Kot wrote:
> On Thu, Nov 1, 2018 at 1:55 PM Valery Kot <valery....@gmail.com> wrote:
>>
>>> 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>.
>>
>> Thanks for the suggestion! Here is an updated patch.
>
> Was too hurry to send a patch, sorry! Here is the correct one.
>
>
> From 638277354338bf42020854e5bebec5fe61677135 Mon Sep 17 00:00:00 2001
> From: vkot <valery....@4cinsights.com>
You might want to fix up this name - I corrected it manually.
> Date: Thu, 1 Nov 2018 14:15:11 +0100
> Subject: [PATCH] Handle sample_aspect_ratio in libopenh264-encoder
>
> ---
> libavcodec/libopenh264enc.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
> diff --git a/libavcodec/libopenh264enc.c b/libavcodec/libopenh264enc.c
> index 83c3f0ce20..b3ddb4609b 100644
> --- a/libavcodec/libopenh264enc.c
> +++ b/libavcodec/libopenh264enc.c
> @@ -164,6 +164,47 @@ 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 && avctx->sample_aspect_ratio.den) {
> + // Table E-1.
> + static const AVRational sar_idc[] = {
> + { 0, 0 }, // Unspecified (never written here).
> + { 1, 1 }, { 12, 11 }, { 10, 11 }, { 16, 11 },
> + { 40, 33 }, { 24, 11 }, { 20, 11 }, { 32, 11 },
> + { 80, 33 }, { 18, 11 }, { 15, 11 }, { 64, 33 },
> + { 160, 99 }, // Last 3 are unknown to openh264: { 4, 3 }, {
> 3, 2 }, { 2, 1 },
> + };
> + static const ESampleAspectRatio asp_idc[] = {
> + ASP_UNSPECIFIED,
> + ASP_1x1, ASP_12x11, ASP_10x11, ASP_16x11,
> + ASP_40x33, ASP_24x11, ASP_20x11, ASP_32x11,
> + ASP_80x33, ASP_18x11, ASP_15x11, ASP_64x33,
> + ASP_160x99,
> + };
> + int num, den, i;
> +
> + av_reduce(&num, &den, avctx->sample_aspect_ratio.num,
> + avctx->sample_aspect_ratio.den, 65535);
> +
> + for (i = 1; i < FF_ARRAY_ELEMS(sar_idc); i++) {
> + if (num == sar_idc[i].num &&
> + den == sar_idc[i].den)
> + break;
> + }
> + if (i == FF_ARRAY_ELEMS(sar_idc)) {
> + param.sSpatialLayers[0].eAspectRatio = ASP_EXT_SAR;
> + param.sSpatialLayers[0].sAspectRatioExtWidth = num;
> + param.sSpatialLayers[0].sAspectRatioExtHeight = den;
> + } else {
> + param.sSpatialLayers[0].eAspectRatio = asp_idc[i];
> + }
> + param.sSpatialLayers[0].bAspectRatioPresent = true;
> + }
> + else {
> + param.sSpatialLayers[0].bAspectRatioPresent = false;
> + }
> +#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
>
LGTM, tested and applied.
(Seems libopenh264 accepts any value in this field, so you can actually just
use the later values they don't provide in the enum. Not sure whether that's
actually an API violation which they might reject in some cases, though...)
Thanks!
- Mark
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
