Re: [FFmpeg-devel] [PATCH] lavf/srt: fix build fail when used the libsrt 1.4.1
On Tue, Jul 14, 2020 at 9:47 PM Moritz Barsnick wrote: > > On Sun, Jul 12, 2020 at 22:44:46 +0800, myp...@gmail.com wrote: > > Maybe I give an inaccurate description in the commit message, in fact, > > libsrt 1.4.1 remove the SRTO_STRICTENC/SRTO_SMOOTHER option, it's will > > lead to FFmpeg build fail when using libsrt version >= 1.4.1 > > I don't mind you description. Yet your code change references 1.4.1: > SRT_VERSION_VALUE >= 0x010401 Will double-check the SRT_VERSION_VALUE, thx > while I am trying to make the point that 1.4.0 also needs this change > (or 1.3.0 in one case, which is already ensured by #if). > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 2/2] libavcodec/libaomenc.c: Add command-line options for inter-coding tools
From: Wang Cao
Signed-off-by: Wang Cao
---
doc/encoders.texi | 36 +
libavcodec/libaomenc.c | 60 ++
libavcodec/version.h | 2 +-
3 files changed, 97 insertions(+), 1 deletion(-)
diff --git a/doc/encoders.texi b/doc/encoders.texi
index e6ef401a9a..1a4dbedff3 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1649,6 +1649,42 @@ Use DCT only for INTER modes. Default is false.
@item use-intra-default-tx-only (@emph{boolean}) (Requires libaom >= v2.0.0)
Use Default-transform only for INTRA modes. Default is false.
+@item enable-ref-frame-mvs (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable temporal mv prediction. Default is true.
+
+@item enable-reduced-reference-set (@emph{boolean}) (Requires libaom >= v2.0.0)
+Use reduced set of single and compound references. Default is false.
+
+@item enable-obmc (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable obmc. Default is true.
+
+@item enable-dual-filter (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable dual filter. Default is true.
+
+@item enable-diff-wtd-comp (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable difference-weighted compound. Default is true.
+
+@item enable-dist-wtd-comp (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable distance-weighted compound. Default is true.
+
+@item enable-onesided-comp (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable one sided compound. Default is true.
+
+@item enable-interinter-wedge (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable interinter wedge compound. Default is true.
+
+@item enable-interintra-wedge (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable interintra wedge compound. Default is true.
+
+@item enable-masked-comp (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable masked compound. Default is true.
+
+@item enable-interintra-comp (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable interintra compound. Default is true.
+
+@item enable-smooth-interintra (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable smooth interintra mode. Default is true.
+
@end table
@section libkvazaar
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index b01e6c7283..745acc7d94 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -112,6 +112,18 @@ typedef struct AOMEncoderContext {
int use_intra_dct_only;
int use_inter_dct_only;
int use_intra_default_tx_only;
+int enable_ref_frame_mvs;
+int enable_interinter_wedge;
+int enable_interintra_wedge;
+int enable_interintra_comp;
+int enable_masked_comp;
+int enable_obmc;
+int enable_onesided_comp;
+int enable_reduced_reference_set;
+int enable_smooth_interintra;
+int enable_diff_wtd_comp;
+int enable_dist_wtd_comp;
+int enable_dual_filter;
} AOMContext;
static const char *const ctlidstr[] = {
@@ -168,6 +180,18 @@ static const char *const ctlidstr[] = {
[AV1E_SET_INTER_DCT_ONLY]= "AV1E_SET_INTER_DCT_ONLY",
[AV1E_SET_INTRA_DEFAULT_TX_ONLY] = "AV1E_SET_INTRA_DEFAULT_TX_ONLY",
[AV1E_SET_REDUCED_TX_TYPE_SET] = "AV1E_SET_REDUCED_TX_TYPE_SET",
+[AV1E_SET_ENABLE_DIFF_WTD_COMP] = "AV1E_SET_ENABLE_DIFF_WTD_COMP",
+[AV1E_SET_ENABLE_DIST_WTD_COMP] = "AV1E_SET_ENABLE_DIST_WTD_COMP",
+[AV1E_SET_ENABLE_DUAL_FILTER] = "AV1E_SET_ENABLE_DUAL_FILTER",
+[AV1E_SET_ENABLE_INTERINTER_WEDGE] = "AV1E_SET_ENABLE_INTERINTER_WEDGE",
+[AV1E_SET_ENABLE_INTERINTRA_WEDGE] = "AV1E_SET_ENABLE_INTERINTRA_WEDGE",
+[AV1E_SET_ENABLE_MASKED_COMP] = "AV1E_SET_ENABLE_MASKED_COMP",
+[AV1E_SET_ENABLE_INTERINTRA_COMP] = "AV1E_SET_ENABLE_INTERINTRA_COMP",
+[AV1E_SET_ENABLE_OBMC] = "AV1E_SET_ENABLE_OBMC",
+[AV1E_SET_ENABLE_ONESIDED_COMP] = "AV1E_SET_ENABLE_ONESIDED_COMP",
+[AV1E_SET_REDUCED_REFERENCE_SET]= "AV1E_SET_REDUCED_REFERENCE_SET",
+[AV1E_SET_ENABLE_SMOOTH_INTERINTRA] = "AV1E_SET_ENABLE_SMOOTH_INTERINTRA",
+[AV1E_SET_ENABLE_REF_FRAME_MVS] = "AV1E_SET_ENABLE_REF_FRAME_MVS",
#endif
};
@@ -765,6 +789,30 @@ static av_cold int aom_init(AVCodecContext *avctx,
codecctl_int(avctx, AV1E_SET_INTRA_DEFAULT_TX_ONLY,
ctx->use_intra_default_tx_only);
if (ctx->reduced_tx_type_set >= 0)
codecctl_int(avctx, AV1E_SET_REDUCED_TX_TYPE_SET,
ctx->reduced_tx_type_set);
+if (ctx->enable_ref_frame_mvs >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_REF_FRAME_MVS,
ctx->enable_ref_frame_mvs);
+if (ctx->enable_reduced_reference_set >= 0)
+codecctl_int(avctx, AV1E_SET_REDUCED_REFERENCE_SET,
ctx->enable_reduced_reference_set);
+if (ctx->enable_diff_wtd_comp >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_DIFF_WTD_COMP,
ctx->enable_diff_wtd_comp);
+if (ctx->enable_dist_wtd_comp >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_DIST_WTD_COMP,
ctx->enable_dist_wtd_comp);
+if (ctx->enable_dual_filter >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_DUAL_FILTER,
[FFmpeg-devel] [PATCH v2 1/2] libavcodec/libaomenc.c: Add command-line options for tx tools.
From: Wang Cao
Signed-off-by: Wang Cao
---
doc/encoders.texi | 20
libavcodec/libaomenc.c | 32
libavcodec/version.h | 2 +-
3 files changed, 53 insertions(+), 1 deletion(-)
diff --git a/doc/encoders.texi b/doc/encoders.texi
index 5406d20c00..e6ef401a9a 100644
--- a/doc/encoders.texi
+++ b/doc/encoders.texi
@@ -1629,6 +1629,26 @@ Enable paeth predictor in intra prediction. Default is
true.
@item enable-palette (@emph{boolean}) (Requires libaom >= v2.0.0)
Enable palette prediction mode. Default is true.
+@item enable-flip-idtx (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable extended transform type, including FLIPADST_DCT, DCT_FLIPADST,
+FLIPADST_FLIPADST, ADST_FLIPADST, FLIPADST_ADST, IDTX, V_DCT, H_DCT,
+V_ADST, H_ADST, V_FLIPADST, H_FLIPADST. Default is true.
+
+@item enable-tx64 (@emph{boolean}) (Requires libaom >= v2.0.0)
+Enable 64-pt transform. Default is true.
+
+@item reduced-tx-type-set (@emph{boolean}) (Requires libaom >= v2.0.0)
+Use reduced set of transform types. Default is false.
+
+@item use-intra-dct-only (@emph{boolean}) (Requires libaom >= v2.0.0)
+Use DCT only for INTRA modes. Default is false.
+
+@item use-inter-dct-only (@emph{boolean}) (Requires libaom >= v2.0.0)
+Use DCT only for INTER modes. Default is false.
+
+@item use-intra-default-tx-only (@emph{boolean}) (Requires libaom >= v2.0.0)
+Use Default-transform only for INTRA modes. Default is false.
+
@end table
@section libkvazaar
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 2ecb3de3a7..b01e6c7283 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -106,6 +106,12 @@ typedef struct AOMEncoderContext {
int enable_intra_edge_filter;
int enable_palette;
int enable_filter_intra;
+int enable_flip_idtx;
+int enable_tx64;
+int reduced_tx_type_set;
+int use_intra_dct_only;
+int use_inter_dct_only;
+int use_intra_default_tx_only;
} AOMContext;
static const char *const ctlidstr[] = {
@@ -156,6 +162,12 @@ static const char *const ctlidstr[] = {
[AV1E_SET_ENABLE_PAETH_INTRA] = "AV1E_SET_ENABLE_PAETH_INTRA",
[AV1E_SET_ENABLE_SMOOTH_INTRA] = "AV1E_SET_ENABLE_SMOOTH_INTRA",
[AV1E_SET_ENABLE_PALETTE] = "AV1E_SET_ENABLE_PALETTE",
+[AV1E_SET_ENABLE_FLIP_IDTX] = "AV1E_SET_ENABLE_FLIP_IDTX",
+[AV1E_SET_ENABLE_TX64] = "AV1E_SET_ENABLE_TX64",
+[AV1E_SET_INTRA_DCT_ONLY]= "AV1E_SET_INTRA_DCT_ONLY",
+[AV1E_SET_INTER_DCT_ONLY]= "AV1E_SET_INTER_DCT_ONLY",
+[AV1E_SET_INTRA_DEFAULT_TX_ONLY] = "AV1E_SET_INTRA_DEFAULT_TX_ONLY",
+[AV1E_SET_REDUCED_TX_TYPE_SET] = "AV1E_SET_REDUCED_TX_TYPE_SET",
#endif
};
@@ -741,6 +753,18 @@ static av_cold int aom_init(AVCodecContext *avctx,
codecctl_int(avctx, AV1E_SET_ENABLE_SMOOTH_INTRA,
ctx->enable_smooth_intra);
if (ctx->enable_palette >= 0)
codecctl_int(avctx, AV1E_SET_ENABLE_PALETTE, ctx->enable_palette);
+if (ctx->enable_tx64 >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_TX64, ctx->enable_tx64);
+if (ctx->enable_flip_idtx >= 0)
+codecctl_int(avctx, AV1E_SET_ENABLE_FLIP_IDTX, ctx->enable_flip_idtx);
+if (ctx->use_intra_dct_only >= 0)
+codecctl_int(avctx, AV1E_SET_INTRA_DCT_ONLY, ctx->use_intra_dct_only);
+if (ctx->use_inter_dct_only >= 0)
+codecctl_int(avctx, AV1E_SET_INTER_DCT_ONLY, ctx->use_inter_dct_only);
+if (ctx->use_intra_default_tx_only >= 0)
+codecctl_int(avctx, AV1E_SET_INTRA_DEFAULT_TX_ONLY,
ctx->use_intra_default_tx_only);
+if (ctx->reduced_tx_type_set >= 0)
+codecctl_int(avctx, AV1E_SET_REDUCED_TX_TYPE_SET,
ctx->reduced_tx_type_set);
#endif
codecctl_int(avctx, AOME_SET_STATIC_THRESHOLD, ctx->static_thresh);
@@ -1152,6 +1176,7 @@ static const AVOption options[] = {
{ "psnr",NULL, 0, AV_OPT_TYPE_CONST, {.i64 =
AOM_TUNE_PSNR}, 0, 0, VE, "tune"},
{ "ssim",NULL, 0, AV_OPT_TYPE_CONST, {.i64 =
AOM_TUNE_SSIM}, 0, 0, VE, "tune"},
FF_AV1_PROFILE_OPTS
+#if AOM_ENCODER_ABI_VERSION >= 22
{ "enable-rect-partitions", "Enable rectangular partitions",
OFFSET(enable_rect_partitions), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
{ "enable-1to4-partitions", "Enable 1:4/4:1 partitions",
OFFSET(enable_1to4_partitions), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
{ "enable-ab-partitions", "Enable ab shape partitions",
OFFSET(enable_ab_partitions), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, VE},
@@ -1162,6 +1187,13 @@ static const AVOption options[] = {
{ "enable-smooth-intra", "Enable smooth intra prediction mode",
OFFSET(enable_smooth_intra), AV_OPT_TYPE_BOOL, {.i64 = -1}, -1,
1, VE},
{ "enable-paeth-intra", "Enable paeth predictor in intra
prediction", OFFSET(enable_paeth_intra), AV_OPT_TYPE_BOOL, {.i64
= -1}, -1, 1, VE},
{
Re: [FFmpeg-devel] [PATCH 3/6] avcodec/h264_slice: export S12M timecode side data
On Wed, Jul 15, 2020 at 12:54:35AM +0100, Kieran Kunhya wrote:
> On Wed, 15 Jul 2020 at 00:36, wrote:
>
> > From: Limin Wang
> >
> > Signed-off-by: Limin Wang
> > ---
> > libavcodec/h264_slice.c | 10 ++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> > index c7b2764..db720de 100644
> > --- a/libavcodec/h264_slice.c
> > +++ b/libavcodec/h264_slice.c
> > @@ -1307,6 +1307,7 @@ static int h264_export_frame_props(H264Context *h)
> > if (h->sei.picture_timing.timecode_cnt > 0) {
> > uint32_t *tc_sd;
> > char tcbuf[AV_TIMECODE_STR_SIZE];
> > +uint32_t *s12m_sd;
> >
> > AVFrameSideData *tcside = av_frame_new_side_data(out,
> >
> > AV_FRAME_DATA_S12M_TIMECODE,
> > @@ -1316,6 +1317,12 @@ static int h264_export_frame_props(H264Context *h)
> >
> > tc_sd = (uint32_t*)tcside->data;
> > tc_sd[0] = h->sei.picture_timing.timecode_cnt;
> > +if (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
> > +s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
> > +if (!s12m_sd)
> > +return AVERROR(ENOMEM);
> > +s12m_sd[0] = tc_sd[0];
> > +}
> >
> > for (int i = 0; i < tc_sd[0]; i++) {
> > int drop = h->sei.picture_timing.timecode[i].dropframe;
> > @@ -1325,6 +1332,9 @@ static int h264_export_frame_props(H264Context *h)
> > int ff = h->sei.picture_timing.timecode[i].frame;
> >
> > tc_sd[i + 1] = av_timecode_get_smpte(h->avctx->framerate,
> > drop, hh, mm, ss, ff);
> > +if (h->avctx->export_side_data &
> > AV_CODEC_EXPORT_DATA_S12M_TC) {
> > +s12m_sd[i + 1] = tc_sd[i + 1];
> > +}
> > av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1], 0);
> > av_dict_set(>metadata, "timecode", tcbuf, 0);
> > }
> > --
>
>
> Does this not duplicate the existing logic?
It's export packet timecode sidedata, so that you can export the TC side data
to stream
level. patch#5 show the command how to use it, before the patch, you had to
input timecode
to export the TC to tmcd track for mp4. Please give advise if you have better
way to get it,
thanks.
>
> Kieran
--
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/6] avcodec/h264_slice: export S12M timecode side data
On Wed, 15 Jul 2020 at 00:36, wrote:
> From: Limin Wang
>
> Signed-off-by: Limin Wang
> ---
> libavcodec/h264_slice.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
> index c7b2764..db720de 100644
> --- a/libavcodec/h264_slice.c
> +++ b/libavcodec/h264_slice.c
> @@ -1307,6 +1307,7 @@ static int h264_export_frame_props(H264Context *h)
> if (h->sei.picture_timing.timecode_cnt > 0) {
> uint32_t *tc_sd;
> char tcbuf[AV_TIMECODE_STR_SIZE];
> +uint32_t *s12m_sd;
>
> AVFrameSideData *tcside = av_frame_new_side_data(out,
>
> AV_FRAME_DATA_S12M_TIMECODE,
> @@ -1316,6 +1317,12 @@ static int h264_export_frame_props(H264Context *h)
>
> tc_sd = (uint32_t*)tcside->data;
> tc_sd[0] = h->sei.picture_timing.timecode_cnt;
> +if (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
> +s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
> +if (!s12m_sd)
> +return AVERROR(ENOMEM);
> +s12m_sd[0] = tc_sd[0];
> +}
>
> for (int i = 0; i < tc_sd[0]; i++) {
> int drop = h->sei.picture_timing.timecode[i].dropframe;
> @@ -1325,6 +1332,9 @@ static int h264_export_frame_props(H264Context *h)
> int ff = h->sei.picture_timing.timecode[i].frame;
>
> tc_sd[i + 1] = av_timecode_get_smpte(h->avctx->framerate,
> drop, hh, mm, ss, ff);
> +if (h->avctx->export_side_data &
> AV_CODEC_EXPORT_DATA_S12M_TC) {
> +s12m_sd[i + 1] = tc_sd[i + 1];
> +}
> av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1], 0);
> av_dict_set(>metadata, "timecode", tcbuf, 0);
> }
> --
Does this not duplicate the existing logic?
Kieran
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 5/6] avformat/movenc: use the stream s12m side data for tmcd track if available
From: Limin Wang
Please test with below command:
./ffmpeg -export_side_data s12m_tc -i
../fate-suite/h264/crew_cif_timecode-2.h264 out.mp4
Signed-off-by: Limin Wang
---
libavformat/movenc.c | 36
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 7db2e28..ca2c8a1 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -6402,12 +6402,25 @@ static int mov_init(AVFormatContext *s)
for (i = 0; i < s->nb_streams; i++) {
AVStream *st = s->streams[i];
AVDictionaryEntry *t = global_tcr;
+AVTimecode tc;
+uint32_t *tc_sd = (uint32_t *)av_stream_get_side_data(st,
AV_PKT_DATA_S12M_TIMECODE,
+ NULL);
+
if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
(t || (t=av_dict_get(st->metadata, "timecode", NULL, 0 {
-AVTimecode tc;
ret = mov_check_timecode_track(s, , i, t->value);
if (ret >= 0)
mov->nb_meta_tmcd++;
+} else if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO &&
tc_sd) {
+char tcbuf[AV_TIMECODE_STR_SIZE];
+int m = tc_sd[0] & 3;
+if ( m > 0) {
+/* use the first timecode if more than one TC */
+av_timecode_make_smpte_tc_string(tcbuf, tc_sd[1], 0);
+ret = mov_check_timecode_track(s, , i, tcbuf);
+if (ret >= 0)
+mov->nb_meta_tmcd++;
+}
}
}
@@ -6724,12 +6737,27 @@ static int mov_write_header(AVFormatContext *s)
if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
AVTimecode tc;
+uint32_t *tc_sd = (uint32_t *)av_stream_get_side_data(st,
AV_PKT_DATA_S12M_TIMECODE,
+ NULL);
if (!t)
t = av_dict_get(st->metadata, "timecode", NULL, 0);
-if (!t)
-continue;
-if (mov_check_timecode_track(s, , i, t->value) < 0)
+if (t) {
+if (mov_check_timecode_track(s, , i, t->value) < 0)
+continue;
+} else if (tc_sd) {
+char tcbuf[AV_TIMECODE_STR_SIZE];
+int m = tc_sd[0] & 3;
+if ( m < 1)
+continue;
+
+/* use the first timecode if more than one */
+av_timecode_make_smpte_tc_string(tcbuf, tc_sd[1], 0);
+ret = mov_check_timecode_track(s, , i, tcbuf);
+if (ret < 0)
+continue;
+} else
continue;
+
if ((ret = mov_create_timecode_track(s, tmcd_track, i, tc)) <
0)
return ret;
tmcd_track++;
--
1.8.3.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 3/6] avcodec/h264_slice: export S12M timecode side data
From: Limin Wang
Signed-off-by: Limin Wang
---
libavcodec/h264_slice.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index c7b2764..db720de 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1307,6 +1307,7 @@ static int h264_export_frame_props(H264Context *h)
if (h->sei.picture_timing.timecode_cnt > 0) {
uint32_t *tc_sd;
char tcbuf[AV_TIMECODE_STR_SIZE];
+uint32_t *s12m_sd;
AVFrameSideData *tcside = av_frame_new_side_data(out,
AV_FRAME_DATA_S12M_TIMECODE,
@@ -1316,6 +1317,12 @@ static int h264_export_frame_props(H264Context *h)
tc_sd = (uint32_t*)tcside->data;
tc_sd[0] = h->sei.picture_timing.timecode_cnt;
+if (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
+s12m_sd = ff_add_s12m_timecode_side_data(h->avctx);
+if (!s12m_sd)
+return AVERROR(ENOMEM);
+s12m_sd[0] = tc_sd[0];
+}
for (int i = 0; i < tc_sd[0]; i++) {
int drop = h->sei.picture_timing.timecode[i].dropframe;
@@ -1325,6 +1332,9 @@ static int h264_export_frame_props(H264Context *h)
int ff = h->sei.picture_timing.timecode[i].frame;
tc_sd[i + 1] = av_timecode_get_smpte(h->avctx->framerate, drop,
hh, mm, ss, ff);
+if (h->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
+s12m_sd[i + 1] = tc_sd[i + 1];
+}
av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1], 0);
av_dict_set(>metadata, "timecode", tcbuf, 0);
}
--
1.8.3.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 6/6] avformat/dv: use the stream s12m side data for timecode track if avaiable
From: Limin Wang
Signed-off-by: Limin Wang
---
libavformat/dvenc.c | 12
1 file changed, 12 insertions(+)
diff --git a/libavformat/dvenc.c b/libavformat/dvenc.c
index b04d604..b5fea8f 100644
--- a/libavformat/dvenc.c
+++ b/libavformat/dvenc.c
@@ -396,6 +396,18 @@ static int dv_write_header(AVFormatContext *s)
}
if (tcr && av_timecode_init_from_string(>tc, rate, tcr->value, s) >=
0)
return 0;
+
+for (int i = 0; i < s->nb_streams; i++) {
+uint32_t *sd = (uint32_t *)av_stream_get_side_data(s->streams[i],
+
AV_PKT_DATA_S12M_TIMECODE, NULL);
+if (sd && sd[0] > 0) {
+char tcbuf[AV_TIMECODE_STR_SIZE];
+/* use the first timecode */
+av_timecode_make_smpte_tc_string(tcbuf, sd[1], 0);
+return av_timecode_init_from_string(>tc, rate, tcbuf, s);
+}
+}
+
return av_timecode_init(>tc, rate, 0, 0, s);
}
--
1.8.3.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/6] avcodec/utils: add ff_add_s12m_timecode_side_data()
From: Limin Wang
Signed-off-by: Limin Wang
---
libavcodec/internal.h | 5 +
libavcodec/utils.c| 30 ++
2 files changed, 35 insertions(+)
diff --git a/libavcodec/internal.h b/libavcodec/internal.h
index 0a1c0a1..e0040fc 100644
--- a/libavcodec/internal.h
+++ b/libavcodec/internal.h
@@ -364,6 +364,11 @@ int ff_decode_frame_props(AVCodecContext *avctx, AVFrame
*frame);
AVCPBProperties *ff_add_cpb_side_data(AVCodecContext *avctx);
/**
+ * Add a S12M timecode side data to an codec context.
+ */
+uint32_t *ff_add_s12m_timecode_side_data(AVCodecContext *avctx);
+
+/**
* Check AVFrame for A53 side data and allocate and fill SEI message with A53
info
*
* @param frame Raw frame to get A53 side data from
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 2ece34f..270c0c4 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -2029,6 +2029,36 @@ AVCPBProperties *ff_add_cpb_side_data(AVCodecContext
*avctx)
return props;
}
+uint32_t *ff_add_s12m_timecode_side_data(AVCodecContext *avctx)
+{
+AVPacketSideData *tmp;
+uint32_t *data;
+int size = sizeof(uint32_t) * 4;
+int i;
+
+for (i = 0; i < avctx->nb_coded_side_data; i++)
+if (avctx->coded_side_data[i].type == AV_PKT_DATA_S12M_TIMECODE)
+return (uint32_t*)avctx->coded_side_data[i].data;
+
+data = av_mallocz(size);
+if (!data)
+return NULL;
+
+tmp = av_realloc_array(avctx->coded_side_data, avctx->nb_coded_side_data +
1, sizeof(*tmp));
+if (!tmp) {
+av_freep();
+return NULL;
+}
+avctx->coded_side_data = tmp;
+
+avctx->coded_side_data[avctx->nb_coded_side_data].type =
AV_PKT_DATA_S12M_TIMECODE;
+avctx->coded_side_data[avctx->nb_coded_side_data].data = (uint8_t *)data;
+avctx->coded_side_data[avctx->nb_coded_side_data].size = size;
+avctx->nb_coded_side_data++;
+
+return data;
+}
+
static void codec_parameters_reset(AVCodecParameters *par)
{
av_freep(>extradata);
--
1.8.3.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 1/6] avcodec: add AV_CODEC_EXPORT_DATA_S12M_TC flag to export S12M timecode
From: Limin Wang
Signed-off-by: Limin Wang
---
doc/APIchanges | 3 +++
doc/codecs.texi| 3 +++
libavcodec/avcodec.h | 4
libavcodec/options_table.h | 1 +
libavcodec/version.h | 2 +-
5 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/doc/APIchanges b/doc/APIchanges
index 67f2ac3..2811ee7 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -15,6 +15,9 @@ libavutil: 2017-10-21
API changes, most recent first:
+2020-07-xx - xx - lavc 58.97.100 - avcodec.h
+ Add AV_CODEC_EXPORT_DATA_S12M_TC.
+
2020-07-xx - xx - lavc 58.96.100 - packet.h
Add AV_PKT_DATA_S12M_TIMECODE.
diff --git a/doc/codecs.texi b/doc/codecs.texi
index c092aad..263da9c 100644
--- a/doc/codecs.texi
+++ b/doc/codecs.texi
@@ -816,6 +816,9 @@ for codecs that support it. See also
@file{doc/examples/export_mvs.c}.
@item prft
Export encoder Producer Reference Time into packet side-data (see
@code{AV_PKT_DATA_PRFT})
for codecs that support it.
+@item s12m_tc
+Export SMPTE ST 12-1 timecode through packet side data (see
@code{AV_PKT_DATA_S12M_TIMECODE})
+for codecs that support it.
@end table
@item error @var{integer} (@emph{encoding,video})
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index c91b2fd..f8420c0 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -410,6 +410,10 @@ typedef struct RcOverride{
* Export the AVVideoEncParams structure through frame side data.
*/
#define AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS (1 << 2)
+/**
+ * Export SMPTE ST 12-1 timecode through packet side data
+ */
+#define AV_CODEC_EXPORT_DATA_S12M_TC (1 << 3)
/**
* Pan Scan area.
diff --git a/libavcodec/options_table.h b/libavcodec/options_table.h
index 8ba137f..dcedfbb 100644
--- a/libavcodec/options_table.h
+++ b/libavcodec/options_table.h
@@ -83,6 +83,7 @@ static const AVOption avcodec_options[] = {
{"mvs", "export motion vectors through frame side data", 0, AV_OPT_TYPE_CONST,
{.i64 = AV_CODEC_EXPORT_DATA_MVS}, INT_MIN, INT_MAX, V|D, "export_side_data"},
{"prft", "export Producer Reference Time through packet side data", 0,
AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_PRFT}, INT_MIN, INT_MAX,
A|V|S|E, "export_side_data"},
{"venc_params", "export video encoding parameters through frame side data", 0,
AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_VIDEO_ENC_PARAMS}, INT_MIN,
INT_MAX, V|D, "export_side_data"},
+{"s12m_tc", "export SMPTE ST 12-1 timecode through packet side data", 0,
AV_OPT_TYPE_CONST, {.i64 = AV_CODEC_EXPORT_DATA_S12M_TC}, INT_MIN, INT_MAX,
V|D, "export_side_data"},
{"time_base", NULL, OFFSET(time_base), AV_OPT_TYPE_RATIONAL, {.dbl = 0}, 0,
INT_MAX},
{"g", "set the group of picture (GOP) size", OFFSET(gop_size),
AV_OPT_TYPE_INT, {.i64 = 12 }, INT_MIN, INT_MAX, V|E},
{"ar", "set audio sampling rate (in Hz)", OFFSET(sample_rate),
AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, A|D|E},
diff --git a/libavcodec/version.h b/libavcodec/version.h
index ad0bfd6..d5dca8c 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -28,7 +28,7 @@
#include "libavutil/version.h"
#define LIBAVCODEC_VERSION_MAJOR 58
-#define LIBAVCODEC_VERSION_MINOR 96
+#define LIBAVCODEC_VERSION_MINOR 97
#define LIBAVCODEC_VERSION_MICRO 100
#define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR, \
--
1.8.3.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 4/6] avcodec/hevcdec: export S12M timecode side data
From: Limin Wang
Signed-off-by: Limin Wang
---
libavcodec/hevcdec.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index b77df8d..6e24c9b 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -2827,6 +2827,7 @@ static int set_side_data(HEVCContext *s)
if (s->sei.timecode.present) {
uint32_t *tc_sd;
char tcbuf[AV_TIMECODE_STR_SIZE];
+uint32_t *s12m_sd;
AVFrameSideData *tcside = av_frame_new_side_data(out,
AV_FRAME_DATA_S12M_TIMECODE,
sizeof(uint32_t) * 4);
if (!tcside)
@@ -2834,6 +2835,12 @@ static int set_side_data(HEVCContext *s)
tc_sd = (uint32_t*)tcside->data;
tc_sd[0] = s->sei.timecode.num_clock_ts;
+if (s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
+s12m_sd = ff_add_s12m_timecode_side_data(s->avctx);
+if (!s12m_sd)
+return AVERROR(ENOMEM);
+s12m_sd[0] = tc_sd[0];
+}
for (int i = 0; i < tc_sd[0]; i++) {
int drop = s->sei.timecode.cnt_dropped_flag[i];
@@ -2843,6 +2850,9 @@ static int set_side_data(HEVCContext *s)
int ff = s->sei.timecode.n_frames[i];
tc_sd[i + 1] = av_timecode_get_smpte(s->avctx->framerate, drop,
hh, mm, ss, ff);
+if (s->avctx->export_side_data & AV_CODEC_EXPORT_DATA_S12M_TC) {
+s12m_sd[i + 1] = tc_sd[i + 1];
+}
av_timecode_make_smpte_tc_string(tcbuf, tc_sd[i + 1], 0);
av_dict_set(>metadata, "timecode", tcbuf, 0);
}
--
1.8.3.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/7] avcodec/golomb, h264*: Fix get_ue_golomb_31()
Michael Niedermayer:
> On Tue, Jul 14, 2020 at 10:43:51PM +0200, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> On Tue, Jul 14, 2020 at 05:34:50PM +0200, Andreas Rheinhardt wrote:
get_ue_golomb_31() uses a LUT of 512 entries; therefore it can be used
to parse exp-golomb codes of length <= 9, i.e. those codes with at most
four leading bits that have five effective bits; this implies a range of
0..30 and not 31. In particular, this function must not be used to parse
e.g. the H.264 SPS id.
>>>
>>> hmm, are you sure ?
>>>
>>
>> Yes.
>>
>>> 1 0
>>> 01X 1-2
>>> 001XX 3-6
>>> 0001XXX 7-14
>>> 1 15-30
>>> 01. 31
>>>
>>> we need to read 9 bits for this, we do not need to read the bits marked with
>>> a "." because the code is already determined at this point.
>>>
>> The code is only determined at the point if one already presumes that it
>> can't be anything >31.
>
> yes, that is the idea of get_ue_golomb_31() its only used when the biggest
> valid code is 31.
>
But then we still have to rely on the code being valid as we have no way
to distinguish 31 from 32-34. Is this considered acceptable for the
speed gain?
>
>> Without this assumption, one needs to look at all
>> the five "." in order to distinguish it from 32..62. By looking at only
>> 9 digits one can not rule out 32-34.
>> And if you take a look at ff_ue_golomb_vlc_code, you will see that it
>> does not even contain 31 at all. It returns the values 0-30 for the
>> codes with length <= 9 and returns 32 for the rest.
>
> const uint8_t ff_ue_golomb_vlc_code[512]={
> 32,32,32,32,32,32,32,32,31,32,32,32,32,32,32,32,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,
> ^^
Indeed, how could I overlook this. Must have been blind. Sorry for that.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/7] avcodec/golomb, h264*: Fix get_ue_golomb_31()
On Tue, Jul 14, 2020 at 10:43:51PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Tue, Jul 14, 2020 at 05:34:50PM +0200, Andreas Rheinhardt wrote:
> >> get_ue_golomb_31() uses a LUT of 512 entries; therefore it can be used
> >> to parse exp-golomb codes of length <= 9, i.e. those codes with at most
> >> four leading bits that have five effective bits; this implies a range of
> >> 0..30 and not 31. In particular, this function must not be used to parse
> >> e.g. the H.264 SPS id.
> >
> > hmm, are you sure ?
> >
>
> Yes.
>
> > 1 0
> > 01X 1-2
> > 001XX 3-6
> > 0001XXX 7-14
> > 1 15-30
> > 01. 31
> >
> > we need to read 9 bits for this, we do not need to read the bits marked with
> > a "." because the code is already determined at this point.
> >
> The code is only determined at the point if one already presumes that it
> can't be anything >31.
yes, that is the idea of get_ue_golomb_31() its only used when the biggest
valid code is 31.
> Without this assumption, one needs to look at all
> the five "." in order to distinguish it from 32..62. By looking at only
> 9 digits one can not rule out 32-34.
> And if you take a look at ff_ue_golomb_vlc_code, you will see that it
> does not even contain 31 at all. It returns the values 0-30 for the
> codes with length <= 9 and returns 32 for the rest.
const uint8_t ff_ue_golomb_vlc_code[512]={
32,32,32,32,32,32,32,32,31,32,32,32,32,32,32,32,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,
^^
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/3] libavutil/imgutils: add utility to get plane sizes
On 7/14/2020 5:49 PM, Michael Niedermayer wrote: > On Tue, Jul 14, 2020 at 11:19:54AM -0300, James Almer wrote: >> On 7/14/2020 9:47 AM, Nicolas George wrote: >>> James Almer (12020-07-10): Because my opinion and tastes are not yours. I already said why *i* consider it ugly. It doesn't need to fit *your* conception of ugliness. >>> >>> If it is only a matter of taste, then it cannot count as an argument. >>> But tastes are frequently a proxy for minor factors. If you can express >>> the minor factors behind your tastes, they can count as arguments. >>> >>> Which is what I am trying to do about my tastes. >>> >>> One of these minor factors: there is frequently a "int ret" variable and >>> a "return ret" at the end. If the return value conflates size with the >>> error code, ret cannot be used, which means some kind of "if (size < 0) >>> ret = size;" need to happen, but would easily be forgotten, initially or >>> in refactoring. >>> int av_image_fill_plane_sizes(size_t sizes[4], enum AVPixelFormat pix_fmt, const ptrdiff_t linesizes[4]); >>> >>> So the question is now: is the total size useful enough to warrant an >>> extra return parameter or not? I have no advice on the question. >> >> Not for an extra parameter (Who doesn't love enterprise-style code full >> of function calls filled with NULL arguments, right?), but yes for using >> the otherwise undefined >0 scope of the return value, which is only >> possible if we keep the sizes as int like the rest of the module, and to >> be honest something i would very much like to do seeing almost every >> other existing function can't be ported to size_t/ptrdiff_t. >> A complete, consistent set of new functions would have to be introduced >> for that purpose. >> >> I'd like to convince Michael about it, since he suggested these types, >> but if i can't then this is the approach most consistent with existing >> size array filling functions. > > Let me phrase my suggestion in a more high level sense > > Currently the functions use int for lots of cases where they should not > ultimately we want the functions to use more correct types for linesize > sizes and so on. > > We need to add these function(s) because we fix a bug. > Can we take a step toward more correct types while doing this in a > way that makes sense ? > > if so that should be done. > > If not (as in its more mess than if its done later in some other way) > then we should not try to do this now > > The idea is just to take a step towards how these functions/API should look > ideally. Its not to make some inconvenient mess The issue is that most existing functions can't be ported to ptrdiff_t/size_t with a mere type switch after a soname bump, which means to make such a move we'd need to introduce an entire set of new imgutils functions with a different design approach for this purpose. This is why i consider it may be a better idea to define this new single function in a consistent way with the existing API, including types and signature, which is what the first iteration of this set attempted to do, and leave the move to proper types for a rewrite of the module. If not, the v3 posted yesterday is currently the best non-int approach. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/3] libavutil/imgutils: add utility to get plane sizes
On Tue, Jul 14, 2020 at 11:19:54AM -0300, James Almer wrote: > On 7/14/2020 9:47 AM, Nicolas George wrote: > > James Almer (12020-07-10): > >> Because my opinion and tastes are not yours. I already said why *i* > >> consider it ugly. It doesn't need to fit *your* conception of ugliness. > > > > If it is only a matter of taste, then it cannot count as an argument. > > But tastes are frequently a proxy for minor factors. If you can express > > the minor factors behind your tastes, they can count as arguments. > > > > Which is what I am trying to do about my tastes. > > > > One of these minor factors: there is frequently a "int ret" variable and > > a "return ret" at the end. If the return value conflates size with the > > error code, ret cannot be used, which means some kind of "if (size < 0) > > ret = size;" need to happen, but would easily be forgotten, initially or > > in refactoring. > > > >> int av_image_fill_plane_sizes(size_t sizes[4], enum AVPixelFormat > >> pix_fmt, const ptrdiff_t linesizes[4]); > > > > So the question is now: is the total size useful enough to warrant an > > extra return parameter or not? I have no advice on the question. > > Not for an extra parameter (Who doesn't love enterprise-style code full > of function calls filled with NULL arguments, right?), but yes for using > the otherwise undefined >0 scope of the return value, which is only > possible if we keep the sizes as int like the rest of the module, and to > be honest something i would very much like to do seeing almost every > other existing function can't be ported to size_t/ptrdiff_t. > A complete, consistent set of new functions would have to be introduced > for that purpose. > > I'd like to convince Michael about it, since he suggested these types, > but if i can't then this is the approach most consistent with existing > size array filling functions. Let me phrase my suggestion in a more high level sense Currently the functions use int for lots of cases where they should not ultimately we want the functions to use more correct types for linesize sizes and so on. We need to add these function(s) because we fix a bug. Can we take a step toward more correct types while doing this in a way that makes sense ? if so that should be done. If not (as in its more mess than if its done later in some other way) then we should not try to do this now The idea is just to take a step towards how these functions/API should look ideally. Its not to make some inconvenient mess thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Whats the most studid thing your enemy could do ? Blow himself up Whats the most studid thing you could do ? Give up your rights and freedom because your enemy blew himself up. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/7] avcodec/golomb, h264*: Fix get_ue_golomb_31()
Michael Niedermayer: > On Tue, Jul 14, 2020 at 05:34:50PM +0200, Andreas Rheinhardt wrote: >> get_ue_golomb_31() uses a LUT of 512 entries; therefore it can be used >> to parse exp-golomb codes of length <= 9, i.e. those codes with at most >> four leading bits that have five effective bits; this implies a range of >> 0..30 and not 31. In particular, this function must not be used to parse >> e.g. the H.264 SPS id. > > hmm, are you sure ? > Yes. > 1 0 > 01X 1-2 > 001XX 3-6 > 0001XXX 7-14 > 1 15-30 > 01. 31 > > we need to read 9 bits for this, we do not need to read the bits marked with > a "." because the code is already determined at this point. > The code is only determined at the point if one already presumes that it can't be anything >31. Without this assumption, one needs to look at all the five "." in order to distinguish it from 32..62. By looking at only 9 digits one can not rule out 32-34. And if you take a look at ff_ue_golomb_vlc_code, you will see that it does not even contain 31 at all. It returns the values 0-30 for the codes with length <= 9 and returns 32 for the rest. - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 3/7] avcodec/golomb, h264*: Fix get_ue_golomb_31()
On Tue, Jul 14, 2020 at 05:34:50PM +0200, Andreas Rheinhardt wrote: > get_ue_golomb_31() uses a LUT of 512 entries; therefore it can be used > to parse exp-golomb codes of length <= 9, i.e. those codes with at most > four leading bits that have five effective bits; this implies a range of > 0..30 and not 31. In particular, this function must not be used to parse > e.g. the H.264 SPS id. hmm, are you sure ? 1 0 01X 1-2 001XX 3-6 0001XXX 7-14 1 15-30 01. 31 we need to read 9 bits for this, we do not need to read the bits marked with a "." because the code is already determined at this point. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/golomb: Prevent shift by negative number
Michael Niedermayer:
> On Tue, Jul 14, 2020 at 06:10:39PM +0200, Andreas Rheinhardt wrote:
>> Michael Niedermayer:
>>> On Mon, Jul 13, 2020 at 09:04:30PM +0200, Tomas Härdin wrote:
fre 2020-07-10 klockan 15:48 +0200 skrev Andreas Rheinhardt:
> This happened in get_ue_golomb() if the cached bitstream reader was
> in
> use, because there was no check to handle the case of the read value
> not being in the range 0..8190.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/golomb.h | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> index 7fd46a91bd..5bfcfe085f 100644
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -66,6 +66,10 @@ static inline int get_ue_golomb(GetBitContext *gb)
> return ff_ue_golomb_vlc_code[buf];
> } else {
> int log = 2 * av_log2(buf) - 31;
> +if (log < 0) {
> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> +return AVERROR_INVALIDDATA;
> +}
How come invalid codes can even be present like this? Seems
inefficient. Or is the decoder wrong? Maybe Michael wants to chime in
here, since he wrote the original implementation.
>>>
>>> The codepath of the non cached one does a check too. It should be
>>> done consistently.
>>
>> Does this imply that you want me to error out for log < 7 although the
>> cached bitstream reader is able to read the number even when log is in
>> the range of 0..6? (The reason for this is that one only gets 25 valid
>> bits in the noncached version.)
>
> is it intended to be valid for this function to be used for longer codes ?
> if not it should fail because otherwise a bug in the code would be missed
> also it would result in fewer differences and easier testing as both
> would behave the same
>
Ok, will change locally.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/golomb: Prevent shift by negative number
On Tue, Jul 14, 2020 at 06:10:39PM +0200, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Mon, Jul 13, 2020 at 09:04:30PM +0200, Tomas Härdin wrote:
> >> fre 2020-07-10 klockan 15:48 +0200 skrev Andreas Rheinhardt:
> >>> This happened in get_ue_golomb() if the cached bitstream reader was
> >>> in
> >>> use, because there was no check to handle the case of the read value
> >>> not being in the range 0..8190.
> >>>
> >>> Signed-off-by: Andreas Rheinhardt
> >>> ---
> >>> libavcodec/golomb.h | 4
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> >>> index 7fd46a91bd..5bfcfe085f 100644
> >>> --- a/libavcodec/golomb.h
> >>> +++ b/libavcodec/golomb.h
> >>> @@ -66,6 +66,10 @@ static inline int get_ue_golomb(GetBitContext *gb)
> >>> return ff_ue_golomb_vlc_code[buf];
> >>> } else {
> >>> int log = 2 * av_log2(buf) - 31;
> >>> +if (log < 0) {
> >>> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> >>> +return AVERROR_INVALIDDATA;
> >>> +}
> >>
> >> How come invalid codes can even be present like this? Seems
> >> inefficient. Or is the decoder wrong? Maybe Michael wants to chime in
> >> here, since he wrote the original implementation.
> >
> > The codepath of the non cached one does a check too. It should be
> > done consistently.
>
> Does this imply that you want me to error out for log < 7 although the
> cached bitstream reader is able to read the number even when log is in
> the range of 0..6? (The reason for this is that one only gets 25 valid
> bits in the noncached version.)
is it intended to be valid for this function to be used for longer codes ?
if not it should fail because otherwise a bug in the code would be missed
also it would result in fewer differences and easier testing as both
would behave the same
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 6/6] avcodec/h264*: Omit potentially wrong values from log messages
get_ue_golomb_30() and get_ue_golomb() (as well as
get_ue_golomb2()) can only parse values within a certain range
correctly; if the parsed value is not within said range, the latter two
functions return AVERROR_INVALIDDATA, while the former returns something
outside of said range (currently 32 for a 0-30 range). So the return
values are good enough to determine whether an exp golomb code in the
desired range has been encountered, but they are not necessarily
correct. Therefore they should not be used in error messages stating
that a certain value (the return value of these functions) is
out-of-range; instead just state the correct range and that the parsed
value is not in said range.
Signed-off-by: Andreas Rheinhardt
---
Is it actually intentional that the SPS.offset_for_ref_frame array has
256 entries, although the specs only allow 255 elements in an SPS?
libavcodec/cavsdec.c | 2 +-
libavcodec/h264_cavlc.c | 14 +++---
libavcodec/h264_parse.c | 4 ++--
libavcodec/h264_parser.c | 2 +-
libavcodec/h264_ps.c | 26 ++
libavcodec/h264_slice.c | 2 +-
6 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 3d4b9cdeff..2857646438 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -615,7 +615,7 @@ static inline int decode_residual_inter(AVSContext *h)
/* get coded block pattern */
int cbp = get_ue_golomb(>gb);
if (cbp > 63U) {
-av_log(h->avctx, AV_LOG_ERROR, "illegal inter cbp %d\n", cbp);
+av_log(h->avctx, AV_LOG_ERROR, "Inter cbp not in 0-63 range\n");
return AVERROR_INVALIDDATA;
}
h->cbp = cbp_tab[cbp][1];
diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c
index 48f0f0689d..af1a3b1370 100644
--- a/libavcodec/h264_cavlc.c
+++ b/libavcodec/h264_cavlc.c
@@ -762,7 +762,7 @@ int ff_h264_decode_mb_cavlc(const H264Context *h,
H264SliceContext *sl)
mb_type--;
decode_intra_mb:
if(mb_type > 25){
-av_log(h->avctx, AV_LOG_ERROR, "mb_type %d in %c slice too large
at %d %d\n", mb_type, av_get_picture_type_char(sl->slice_type), sl->mb_x,
sl->mb_y);
+av_log(h->avctx, AV_LOG_ERROR, "mb_type in %c slice too large at
%d %d\n", av_get_picture_type_char(sl->slice_type), sl->mb_x, sl->mb_y);
return -1;
}
partition_count=0;
@@ -891,7 +891,7 @@ decode_intra_mb:
}else{
tmp = get_ue_golomb(>gb);
if(tmp>=ref_count){
-av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", tmp);
+av_log(h->avctx, AV_LOG_ERROR, "ref overflow\n");
return -1;
}
}
@@ -967,7 +967,7 @@ decode_intra_mb:
}else{
val = get_ue_golomb(>gb);
if (val >= rc) {
-av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
+av_log(h->avctx, AV_LOG_ERROR, "ref
overflow\n");
return -1;
}
}
@@ -998,7 +998,7 @@ decode_intra_mb:
}else{
val = get_ue_golomb(>gb);
if (val >= rc) {
-av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
+av_log(h->avctx, AV_LOG_ERROR, "ref
overflow\n");
return -1;
}
}
@@ -1036,7 +1036,7 @@ decode_intra_mb:
}else{
val = get_ue_golomb(>gb);
if (val >= rc) {
-av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
+av_log(h->avctx, AV_LOG_ERROR, "ref
overflow\n");
return -1;
}
}
@@ -1071,7 +1071,7 @@ decode_intra_mb:
if(decode_chroma){
if(cbp > 47){
-av_log(h->avctx, AV_LOG_ERROR, "cbp too large (%u) at %d
%d\n", cbp, sl->mb_x, sl->mb_y);
+av_log(h->avctx, AV_LOG_ERROR, "cbp >%d at %d %d\n", 47,
sl->mb_x, sl->mb_y);
return -1;
}
if (IS_INTRA4x4(mb_type))
@@ -1080,7 +1080,7 @@ decode_intra_mb:
cbp = ff_h264_golomb_to_inter_cbp[cbp];
}else{
if(cbp > 15){
-av_log(h->avctx, AV_LOG_ERROR, "cbp too large (%u) at %d
%d\n", cbp, sl->mb_x, sl->mb_y);
+av_log(h->avctx, AV_LOG_ERROR, "cbp >%d at %d %d\n", 15,
sl->mb_x, sl->mb_y);
return -1;
}
[FFmpeg-devel] [PATCH v2 5/6] avcodec/cavsdec, h264*, hevc_parser: Use get_ue_golomb_30 where possible
instead of get_ue_golomb(). The difference between the two is that the
latter also has to take into account the case in which the read code is
more than 9 bits (four preceding zeroes + at most five value bits) long.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cavsdec.c | 2 +-
libavcodec/h264_parse.c | 4 ++--
libavcodec/h264_ps.c | 22 +++---
libavcodec/hevc_parser.c | 2 +-
4 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index aaed807196..3d4b9cdeff 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -676,7 +676,7 @@ static int decode_mb_i(AVSContext *h, int cbp_code)
}
h->pred_mode_Y[pos] = predpred;
}
-pred_mode_uv = get_ue_golomb(gb);
+pred_mode_uv = get_ue_golomb_30(gb);
if (pred_mode_uv > 6) {
av_log(h->avctx, AV_LOG_ERROR, "illegal intra chroma pred mode\n");
return AVERROR_INVALIDDATA;
diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index 352ffea948..53e644da66 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -35,7 +35,7 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS
*sps,
pwt->use_weight = 0;
pwt->use_weight_chroma = 0;
-pwt->luma_log2_weight_denom = get_ue_golomb(gb);
+pwt->luma_log2_weight_denom = get_ue_golomb_30(gb);
if (pwt->luma_log2_weight_denom > 7U) {
av_log(logctx, AV_LOG_ERROR, "luma_log2_weight_denom %d is out of
range\n", pwt->luma_log2_weight_denom);
pwt->luma_log2_weight_denom = 0;
@@ -43,7 +43,7 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS
*sps,
luma_def = 1 << pwt->luma_log2_weight_denom;
if (sps->chroma_format_idc) {
-pwt->chroma_log2_weight_denom = get_ue_golomb(gb);
+pwt->chroma_log2_weight_denom = get_ue_golomb_30(gb);
if (pwt->chroma_log2_weight_denom > 7U) {
av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom %d is out
of range\n", pwt->chroma_log2_weight_denom);
pwt->chroma_log2_weight_denom = 0;
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index c1a79bde53..57ee74ec32 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -181,8 +181,8 @@ static inline int decode_vui_parameters(GetBitContext *gb,
void *logctx,
/* chroma_location_info_present_flag */
if (get_bits1(gb)) {
/* chroma_sample_location_type_top_field */
-sps->chroma_location = get_ue_golomb(gb) + 1;
-get_ue_golomb(gb); /* chroma_sample_location_type_bottom_field */
+sps->chroma_location = get_ue_golomb_30(gb) + 1;
+get_ue_golomb_30(gb); /* chroma_sample_location_type_bottom_field */
} else
sps->chroma_location = AVCHROMA_LOC_LEFT;
@@ -224,11 +224,11 @@ static inline int decode_vui_parameters(GetBitContext
*gb, void *logctx,
sps->bitstream_restriction_flag = get_bits1(gb);
if (sps->bitstream_restriction_flag) {
get_bits1(gb); /* motion_vectors_over_pic_boundaries_flag */
-get_ue_golomb(gb); /* max_bytes_per_pic_denom */
-get_ue_golomb(gb); /* max_bits_per_mb_denom */
-get_ue_golomb(gb); /* log2_max_mv_length_horizontal */
-get_ue_golomb(gb); /* log2_max_mv_length_vertical */
-sps->num_reorder_frames = get_ue_golomb(gb);
+get_ue_golomb_30(gb); /* max_bytes_per_pic_denom */
+get_ue_golomb_30(gb); /* max_bits_per_mb_denom */
+get_ue_golomb_30(gb); /* log2_max_mv_length_horizontal */
+get_ue_golomb_30(gb); /* log2_max_mv_length_vertical */
+sps->num_reorder_frames = get_ue_golomb_30(gb);
get_ue_golomb(gb); /*max_dec_frame_buffering*/
if (get_bits_left(gb) < 0) {
@@ -403,8 +403,8 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb,
AVCodecContext *avctx,
goto fail;
}
}
-sps->bit_depth_luma = get_ue_golomb(gb) + 8;
-sps->bit_depth_chroma = get_ue_golomb(gb) + 8;
+sps->bit_depth_luma = get_ue_golomb_30(gb) + 8;
+sps->bit_depth_chroma = get_ue_golomb_30(gb) + 8;
if (sps->bit_depth_chroma != sps->bit_depth_luma) {
avpriv_request_sample(avctx,
"Different chroma and luma bit depth");
@@ -428,7 +428,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb,
AVCodecContext *avctx,
sps->bit_depth_chroma = 8;
}
-log2_max_frame_num_minus4 = get_ue_golomb(gb);
+log2_max_frame_num_minus4 = get_ue_golomb_30(gb);
if (log2_max_frame_num_minus4 < MIN_LOG2_MAX_FRAME_NUM - 4 ||
log2_max_frame_num_minus4 > MAX_LOG2_MAX_FRAME_NUM - 4) {
av_log(avctx, AV_LOG_ERROR,
@@ -441,7 +441,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb,
AVCodecContext *avctx,
sps->poc_type = get_ue_golomb_30(gb);
if (sps->poc_type == 0) { // FIXME #define
-unsigned t =
[FFmpeg-devel] [PATCH v2 4/6] avcodec/golomb: Document return value of get_ue_golomb_30 on error
If the return value is undefined if the value of the encountered
exp golomb code was outside the 0..30 range, said function can't
be used anywhere where one wants to check for correctness. So document
that in this case the return value is outside the range 0..30 (it
currently always returns 32 in this case).
Signed-off-by: Andreas Rheinhardt
---
libavcodec/golomb.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 775ac9878d..356df2cd92 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -115,7 +115,8 @@ static inline unsigned get_ue_golomb_long(GetBitContext *gb)
/**
* Read unsigned exp golomb code, constraint to a max of 30.
- * the return value is undefined if the stored value exceeds 30.
+ * The return value is outside 0..30 if no exp golomb code
+ * in that range was encountered.
*/
static inline int get_ue_golomb_30(GetBitContext *gb)
{
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 3/6] avcodec/golomb, h264*: Fix get_ue_golomb_31()
get_ue_golomb_31() uses a LUT of 512 entries; therefore it can be used
to parse exp-golomb codes of length <= 9, i.e. those codes with at most
four leading bits that have five effective bits; this implies a range of
0..30 and not 31. In particular, this function must not be used to parse
e.g. the H.264 SPS id.
This commit renames the function to get_ue_golomb_30() and uses it
whereever possible. In places where it is not possible, it has been
replaced by get_ue_golomb2(). Given that the returned value of said
function can be a negative error code, it is no longer included in
av_log() statements (the parsed value can actually also be wrong when
using get_ue_golomb30()).
Signed-off-by: Andreas Rheinhardt
---
libavcodec/golomb.h | 6 +++---
libavcodec/h264_cavlc.c | 14 +++---
libavcodec/h264_parser.c | 8
libavcodec/h264_ps.c | 18 +-
libavcodec/h264_refs.c | 6 +++---
libavcodec/h264_sei.c| 4 ++--
libavcodec/h264_slice.c | 6 +++---
libavformat/mxfenc.c | 2 +-
8 files changed, 32 insertions(+), 32 deletions(-)
diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index a53486d7cf..775ac9878d 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -114,10 +114,10 @@ static inline unsigned get_ue_golomb_long(GetBitContext
*gb)
}
/**
- * read unsigned exp golomb code, constraint to a max of 31.
- * the return value is undefined if the stored value exceeds 31.
+ * Read unsigned exp golomb code, constraint to a max of 30.
+ * the return value is undefined if the stored value exceeds 30.
*/
-static inline int get_ue_golomb_31(GetBitContext *gb)
+static inline int get_ue_golomb_30(GetBitContext *gb)
{
unsigned int buf;
diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c
index 6481992e58..48f0f0689d 100644
--- a/libavcodec/h264_cavlc.c
+++ b/libavcodec/h264_cavlc.c
@@ -838,7 +838,7 @@ decode_intra_mb:
}
if(decode_chroma){
pred_mode= ff_h264_check_intra_pred_mode(h->avctx,
sl->top_samples_available,
-
sl->left_samples_available, get_ue_golomb_31(>gb), 1);
+
sl->left_samples_available, get_ue_golomb_30(>gb), 1);
if(pred_mode < 0)
return -1;
sl->chroma_pred_mode = pred_mode;
@@ -850,7 +850,7 @@ decode_intra_mb:
if (sl->slice_type_nos == AV_PICTURE_TYPE_B) {
for(i=0; i<4; i++){
-sl->sub_mb_type[i]= get_ue_golomb_31(>gb);
+sl->sub_mb_type[i] = get_ue_golomb_30(>gb);
if(sl->sub_mb_type[i] >=13){
av_log(h->avctx, AV_LOG_ERROR, "B sub_mb_type %u out of
range at %d %d\n", sl->sub_mb_type[i], sl->mb_x, sl->mb_y);
return -1;
@@ -868,7 +868,7 @@ decode_intra_mb:
}else{
av_assert2(sl->slice_type_nos == AV_PICTURE_TYPE_P); //FIXME SP
correct ?
for(i=0; i<4; i++){
-sl->sub_mb_type[i]= get_ue_golomb_31(>gb);
+sl->sub_mb_type[i] = get_ue_golomb_30(>gb);
if(sl->sub_mb_type[i] >=4){
av_log(h->avctx, AV_LOG_ERROR, "P sub_mb_type %u out of
range at %d %d\n", sl->sub_mb_type[i], sl->mb_x, sl->mb_y);
return -1;
@@ -889,7 +889,7 @@ decode_intra_mb:
}else if(ref_count == 2){
tmp= get_bits1(>gb)^1;
}else{
-tmp= get_ue_golomb_31(>gb);
+tmp = get_ue_golomb(>gb);
if(tmp>=ref_count){
av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", tmp);
return -1;
@@ -965,7 +965,7 @@ decode_intra_mb:
} else if (rc == 2) {
val= get_bits1(>gb)^1;
}else{
-val= get_ue_golomb_31(>gb);
+val = get_ue_golomb(>gb);
if (val >= rc) {
av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
return -1;
@@ -996,7 +996,7 @@ decode_intra_mb:
} else if (rc == 2) {
val= get_bits1(>gb)^1;
}else{
-val= get_ue_golomb_31(>gb);
+val = get_ue_golomb(>gb);
if (val >= rc) {
av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
return -1;
@@ -1034,7 +1034,7 @@ decode_intra_mb:
} else if (rc == 2) {
val= get_bits1(>gb)^1;
}else{
-val= get_ue_golomb_31(>gb);
+
[FFmpeg-devel] [PATCH v2 2/6] avcodec/golomb: Prevent shift by negative number
This happened in get_ue_golomb() if the cached bitstream reader was in
use, because there was no check to handle the case of the read value
not being in the supported range.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/golomb.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 1f988d74aa..a53486d7cf 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -66,6 +66,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
return ff_ue_golomb_vlc_code[buf];
} else {
int log = 2 * av_log2(buf) - 31;
+if (log < 0)
+return AVERROR_INVALIDDATA;
buf >>= log;
buf--;
skip_bits_long(gb, 32 - log);
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 1/6] avcodec/golomb: Don't emit error message in get_ue_golomb
Said error message is not very informative and lacked a proper logging
context; furthermore, many callers already provided more descriptive
error messages of their own. So just drop this one.
Suggested-by: James Almer
Signed-off-by: Andreas Rheinhardt
---
libavcodec/golomb.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 7fd46a91bd..1f988d74aa 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -87,10 +87,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
int log = 2 * av_log2(buf) - 31;
LAST_SKIP_BITS(re, gb, 32 - log);
CLOSE_READER(re, gb);
-if (log < 7) {
-av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
+if (log < 7)
return AVERROR_INVALIDDATA;
-}
buf >>= log;
buf--;
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/libssh: add AVOptions for authentication
Actually, forget this patch. It was modeled after the previous AVOption patch I sent for the ftp protocol, but a better more generic approach would be to have some sort of callback for API users so they can just implement .netrc. Like this, with just AVOption for usernames and passwords, we can't really implement it for HTTP due to a request leading to redirects which can then change the domain to something else. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/4] libavcodec/jpeg2000dec : Prevent overriding SOP marker bit
On Tue, Jul 14, 2020 at 10:13:13PM +0530, gautamr...@gmail.com wrote: > From: Gautam Ramakrishnan > > Currently, the COC marker overrides the SOP marker bit. > However, only the COD marker may set this value. This > patch fixes this bug. > --- > libavcodec/jpeg2000dec.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/2] swscale/tests/swscale: Initialize res to a non random error code
On Tue, Jul 14, 2020 at 10:44:19AM +0300, Martin Storsjö wrote:
> On Sun, 12 Jul 2020, Michael Niedermayer wrote:
>
> > Regression since: 3adffab073bc59af39035168ac72bc9ffde3
> >
> > -1 is consistent what other error paths return
> >
> > Signed-off-by: Michael Niedermayer
> > ---
> > libswscale/tests/swscale.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libswscale/tests/swscale.c b/libswscale/tests/swscale.c
> > index 58870fdb78..845ced61bb 100644
> > --- a/libswscale/tests/swscale.c
> > +++ b/libswscale/tests/swscale.c
> > @@ -423,8 +423,10 @@ bad_option:
> > for (x = 0; x < W * 4; x++)
> > rgb_data[ x + y * 4 * W] = av_lfg_get();
> > res = sws_scale(sws, rgb_src, rgb_stride, 0, H / 12, (uint8_t * const
> > *) src, stride);
> > -if (res < 0 || res != H)
> > +if (res < 0 || res != H) {
> > +res = -1;
> > goto error;
> > +}
> > sws_freeContext(sws);
> > av_free(rgb_data);
>
> LGTM
will apply
>
> Do note that for the return value of a process is limited to the range 0-255
> normally, and if returning a negative value, it gets mapped to the upper
> range (128-255) which normally is used for when a process is killed by a
> signal. But this issue seems to already be present within this test app, and
> this change makes it internally consistent at least.
yes this should be changed
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] swscale/tests/swscale: Fix incorrect return code check
On Tue, Jul 14, 2020 at 10:42:24AM +0300, Martin Storsjö wrote: > On Sun, 12 Jul 2020, Michael Niedermayer wrote: > > > Regression since: 3adffab073bc59af39035168ac72bc9ffde3 > > > > Signed-off-by: Michael Niedermayer > > --- > > libswscale/tests/swscale.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/libswscale/tests/swscale.c b/libswscale/tests/swscale.c > > index 693f439bf5..58870fdb78 100644 > > --- a/libswscale/tests/swscale.c > > +++ b/libswscale/tests/swscale.c > > @@ -423,7 +423,7 @@ bad_option: > > for (x = 0; x < W * 4; x++) > > rgb_data[ x + y * 4 * W] = av_lfg_get(); > > res = sws_scale(sws, rgb_src, rgb_stride, 0, H / 12, (uint8_t * const > > *) src, stride); > > -if (res < 0 || res != (H / 12)) > > +if (res < 0 || res != H) > > goto error; > > LGTM, this seems to fix the test on both aarch64 and x86. will apply thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Whats the most studid thing your enemy could do ? Blow himself up Whats the most studid thing you could do ? Give up your rights and freedom because your enemy blew himself up. signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/libssh: add AVOptions for authentication
James Almer (12020-07-14): > Perhaps this should instead be implemented as an AV_OPT_TYPE_DICT option > that can accept anything you would otherwise pass as part of the url, so > user, pass, port, etc. It makes things more complicated for us (accessing a dictionary instead of options directly), for API users (setting the dictionary plus the option itself) and for command-line users (extra level of escaping). Unless we expect the list of things obtained from the URL to grow in the future, which is unlikely to say the least, I see no benefit compared to just adding all this at once. Am I missing something? What benefit would you expect? On the other hand, since a lot of protocols do some kind of av_url_split(), it makes sense to factor it. Why not add all these fields to URLContext and have ffurl_connect() split the URL, using options if they are available? Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/libssh: add AVOptions for authentication
On 7/14/2020 3:06 PM, Nicolas Frattaroli wrote:
> This introduces two new AVOption options for the SFTP protocol,
> one named sftp_user to supply the username to be used for auth,
> one named sftp_password to supply the password to be used for auth.
>
> These are useful for when an API user does not wish to deal with
> URL manipulation and percent encoding.
>
> Setting them while also having credentials in the URL will use the
> credentials from the URL. The rationale for this is that credentials
> embedded in the URL are probably more specific to what the user is
> trying to do than anything set by some API user.
>
> Signed-off-by: Nicolas Frattaroli
> ---
> doc/protocols.texi | 8
> libavformat/libssh.c | 17 +
> 2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/doc/protocols.texi b/doc/protocols.texi
> index 64ad3f05d6..40c8c572e5 100644
> --- a/doc/protocols.texi
> +++ b/doc/protocols.texi
> @@ -880,6 +880,14 @@ truncating. Default value is 1.
> Specify the path of the file containing private key to use during
> authorization.
> By default libssh searches for keys in the @file{~/.ssh/} directory.
>
> +@item sftp_user
> +Set a user to be used for authenticating to the SSH daemon. This is
> overridden by the
> +user in the SFTP URL.
> +
> +@item sftp_password
> +Set a password to be used for authenticating to the SSH daemon. This is
> overridden by
> +the password in the SFTP URL.
> +
> @end table
>
> Example: Play a file stored on remote server.
> diff --git a/libavformat/libssh.c b/libavformat/libssh.c
> index 21474f0f0a..a673e49a3a 100644
> --- a/libavformat/libssh.c
> +++ b/libavformat/libssh.c
> @@ -39,6 +39,8 @@ typedef struct {
> int rw_timeout;
> int trunc;
> char *priv_key;
> +const char *option_user;
> +const char *option_password;
> } LIBSSHContext;
>
> static av_cold int libssh_create_ssh_session(LIBSSHContext *libssh, const
> char* hostname, unsigned int port)
> @@ -192,13 +194,13 @@ static av_cold int libssh_close(URLContext *h)
> static av_cold int libssh_connect(URLContext *h, const char *url, char
> *path, size_t path_size)
> {
> LIBSSHContext *libssh = h->priv_data;
> -char proto[10], hostname[1024], credencials[1024];
> +char proto[10], hostname[1024], credentials[1024];
> int port = 22, ret;
> const char *user = NULL, *pass = NULL;
> char *end = NULL;
>
> av_url_split(proto, sizeof(proto),
> - credencials, sizeof(credencials),
> + credentials, sizeof(credentials),
Cosmetic changes should be in a separate commit.
> hostname, sizeof(hostname),
> ,
> path, path_size,
> @@ -214,8 +216,13 @@ static av_cold int libssh_connect(URLContext *h, const
> char *url, char *path, si
> if ((ret = libssh_create_ssh_session(libssh, hostname, port)) < 0)
> return ret;
>
> -user = av_strtok(credencials, ":", );
> -pass = av_strtok(end, ":", );
> +if(!*credentials) {
> +user = libssh->option_user;
> +pass = libssh->option_password;
> +} else {
> +user = av_strtok(credentials, ":", );
> +pass = av_strtok(end, ":", );
> +}
>
> if ((ret = libssh_authentication(libssh, user, pass)) < 0)
> return ret;
> @@ -479,6 +486,8 @@ static const AVOption options[] = {
> {"timeout", "set timeout of socket I/O operations", OFFSET(rw_timeout),
> AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, D|E },
> {"truncate", "Truncate existing files on write", OFFSET(trunc),
> AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, E },
> {"private_key", "set path to private key", OFFSET(priv_key),
> AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D|E },
> +{"sftp_user", "user for SFTP login. Overridden by whatever is in the
> URL.", OFFSET(option_user), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D|E },
> +{"sftp_password", "password for SFTP login. Overridden by whatever is in
> the URL.", OFFSET(option_password), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D|E },
Perhaps this should instead be implemented as an AV_OPT_TYPE_DICT option
that can accept anything you would otherwise pass as part of the url, so
user, pass, port, etc.
> {NULL}
> };
>
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/libssh: add AVOptions for authentication
Thanks for the patch. I thnink it is a good idea. See preliminary
comments below.
I do not know if Lukasz, who wrote this file, still reads the list. If
he does not appear in the next few days, try Ccing him.
Nicolas Frattaroli (12020-07-14):
> This introduces two new AVOption options for the SFTP protocol,
> one named sftp_user to supply the username to be used for auth,
> one named sftp_password to supply the password to be used for auth.
Since the options are specific to the protocol, there is probably no
need to namespace them.
> These are useful for when an API user does not wish to deal with
> URL manipulation and percent encoding.
>
> Setting them while also having credentials in the URL will use the
> credentials from the URL. The rationale for this is that credentials
> embedded in the URL are probably more specific to what the user is
> trying to do than anything set by some API user.
I am not really comfortable with that. I think returning an error for
conflicting options would be best.
>
> Signed-off-by: Nicolas Frattaroli
> ---
> doc/protocols.texi | 8
> libavformat/libssh.c | 17 +
> 2 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/doc/protocols.texi b/doc/protocols.texi
> index 64ad3f05d6..40c8c572e5 100644
> --- a/doc/protocols.texi
> +++ b/doc/protocols.texi
> @@ -880,6 +880,14 @@ truncating. Default value is 1.
> Specify the path of the file containing private key to use during
> authorization.
> By default libssh searches for keys in the @file{~/.ssh/} directory.
>
> +@item sftp_user
> +Set a user to be used for authenticating to the SSH daemon. This is
> overridden by the
> +user in the SFTP URL.
> +
> +@item sftp_password
> +Set a password to be used for authenticating to the SSH daemon. This is
> overridden by
> +the password in the SFTP URL.
> +
> @end table
>
> Example: Play a file stored on remote server.
> diff --git a/libavformat/libssh.c b/libavformat/libssh.c
> index 21474f0f0a..a673e49a3a 100644
> --- a/libavformat/libssh.c
> +++ b/libavformat/libssh.c
> @@ -39,6 +39,8 @@ typedef struct {
> int rw_timeout;
> int trunc;
> char *priv_key;
> +const char *option_user;
> +const char *option_password;
> } LIBSSHContext;
>
> static av_cold int libssh_create_ssh_session(LIBSSHContext *libssh, const
> char* hostname, unsigned int port)
> @@ -192,13 +194,13 @@ static av_cold int libssh_close(URLContext *h)
> static av_cold int libssh_connect(URLContext *h, const char *url, char
> *path, size_t path_size)
> {
> LIBSSHContext *libssh = h->priv_data;
> -char proto[10], hostname[1024], credencials[1024];
> +char proto[10], hostname[1024], credentials[1024];
Usually, we require this kind of change to be in a separate patch. But
for such a minimal change I think we could dispense.
> int port = 22, ret;
> const char *user = NULL, *pass = NULL;
> char *end = NULL;
>
> av_url_split(proto, sizeof(proto),
> - credencials, sizeof(credencials),
> + credentials, sizeof(credentials),
> hostname, sizeof(hostname),
> ,
> path, path_size,
> @@ -214,8 +216,13 @@ static av_cold int libssh_connect(URLContext *h, const
> char *url, char *path, si
> if ((ret = libssh_create_ssh_session(libssh, hostname, port)) < 0)
> return ret;
>
> -user = av_strtok(credencials, ":", );
> -pass = av_strtok(end, ":", );
> +if(!*credentials) {
Nit: spaces between if and (.
> +user = libssh->option_user;
> +pass = libssh->option_password;
> +} else {
> +user = av_strtok(credentials, ":", );
> +pass = av_strtok(end, ":", );
> +}
>
> if ((ret = libssh_authentication(libssh, user, pass)) < 0)
> return ret;
> @@ -479,6 +486,8 @@ static const AVOption options[] = {
> {"timeout", "set timeout of socket I/O operations", OFFSET(rw_timeout),
> AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, D|E },
> {"truncate", "Truncate existing files on write", OFFSET(trunc),
> AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, E },
> {"private_key", "set path to private key", OFFSET(priv_key),
> AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D|E },
> +{"sftp_user", "user for SFTP login. Overridden by whatever is in the
> URL.", OFFSET(option_user), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D|E },
> +{"sftp_password", "password for SFTP login. Overridden by whatever is in
> the URL.", OFFSET(option_password), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D|E },
> {NULL}
> };
>
Regards,
--
Nicolas George
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH] avformat/libssh: add AVOptions for authentication
This introduces two new AVOption options for the SFTP protocol,
one named sftp_user to supply the username to be used for auth,
one named sftp_password to supply the password to be used for auth.
These are useful for when an API user does not wish to deal with
URL manipulation and percent encoding.
Setting them while also having credentials in the URL will use the
credentials from the URL. The rationale for this is that credentials
embedded in the URL are probably more specific to what the user is
trying to do than anything set by some API user.
Signed-off-by: Nicolas Frattaroli
---
doc/protocols.texi | 8
libavformat/libssh.c | 17 +
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/doc/protocols.texi b/doc/protocols.texi
index 64ad3f05d6..40c8c572e5 100644
--- a/doc/protocols.texi
+++ b/doc/protocols.texi
@@ -880,6 +880,14 @@ truncating. Default value is 1.
Specify the path of the file containing private key to use during
authorization.
By default libssh searches for keys in the @file{~/.ssh/} directory.
+@item sftp_user
+Set a user to be used for authenticating to the SSH daemon. This is overridden
by the
+user in the SFTP URL.
+
+@item sftp_password
+Set a password to be used for authenticating to the SSH daemon. This is
overridden by
+the password in the SFTP URL.
+
@end table
Example: Play a file stored on remote server.
diff --git a/libavformat/libssh.c b/libavformat/libssh.c
index 21474f0f0a..a673e49a3a 100644
--- a/libavformat/libssh.c
+++ b/libavformat/libssh.c
@@ -39,6 +39,8 @@ typedef struct {
int rw_timeout;
int trunc;
char *priv_key;
+const char *option_user;
+const char *option_password;
} LIBSSHContext;
static av_cold int libssh_create_ssh_session(LIBSSHContext *libssh, const
char* hostname, unsigned int port)
@@ -192,13 +194,13 @@ static av_cold int libssh_close(URLContext *h)
static av_cold int libssh_connect(URLContext *h, const char *url, char *path,
size_t path_size)
{
LIBSSHContext *libssh = h->priv_data;
-char proto[10], hostname[1024], credencials[1024];
+char proto[10], hostname[1024], credentials[1024];
int port = 22, ret;
const char *user = NULL, *pass = NULL;
char *end = NULL;
av_url_split(proto, sizeof(proto),
- credencials, sizeof(credencials),
+ credentials, sizeof(credentials),
hostname, sizeof(hostname),
,
path, path_size,
@@ -214,8 +216,13 @@ static av_cold int libssh_connect(URLContext *h, const
char *url, char *path, si
if ((ret = libssh_create_ssh_session(libssh, hostname, port)) < 0)
return ret;
-user = av_strtok(credencials, ":", );
-pass = av_strtok(end, ":", );
+if(!*credentials) {
+user = libssh->option_user;
+pass = libssh->option_password;
+} else {
+user = av_strtok(credentials, ":", );
+pass = av_strtok(end, ":", );
+}
if ((ret = libssh_authentication(libssh, user, pass)) < 0)
return ret;
@@ -479,6 +486,8 @@ static const AVOption options[] = {
{"timeout", "set timeout of socket I/O operations", OFFSET(rw_timeout),
AV_OPT_TYPE_INT, {.i64 = -1}, -1, INT_MAX, D|E },
{"truncate", "Truncate existing files on write", OFFSET(trunc),
AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, E },
{"private_key", "set path to private key", OFFSET(priv_key),
AV_OPT_TYPE_STRING, { .str = NULL }, 0, 0, D|E },
+{"sftp_user", "user for SFTP login. Overridden by whatever is in the
URL.", OFFSET(option_user), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D|E },
+{"sftp_password", "password for SFTP login. Overridden by whatever is in
the URL.", OFFSET(option_password), AV_OPT_TYPE_STRING, { 0 }, 0, 0, D|E },
{NULL}
};
--
2.27.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-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/7] avcodec/golomb: Make emitting error message in get_ue_golomb optional
James Almer:
> On 7/14/2020 12:34 PM, Andreas Rheinhardt wrote:
>> This is designed for scenarios where the caller already checks that the
>> returned value is within a certain allowed range and returns an error
>> message if not.
>>
>> Signed-off-by: Andreas Rheinhardt
>> ---
>> libavcodec/golomb.h | 32
>> 1 file changed, 24 insertions(+), 8 deletions(-)
>>
>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
>> index 5bfcfe085f..63069f63e5 100644
>> --- a/libavcodec/golomb.h
>> +++ b/libavcodec/golomb.h
>> @@ -47,12 +47,7 @@ extern const uint8_t
>> ff_interleaved_ue_golomb_vlc_code[256];
>> extern const int8_t ff_interleaved_se_golomb_vlc_code[256];
>> extern const uint8_t ff_interleaved_dirac_golomb_vlc_code[256];
>>
>> -/**
>> - * Read an unsigned Exp-Golomb code in the range 0 to 8190.
>> - *
>> - * @returns the read value or a negative error code.
>> - */
>> -static inline int get_ue_golomb(GetBitContext *gb)
>> +static inline int get_ue_golomb_internal(GetBitContext *gb, int
>> emit_error_msg)
>> {
>> unsigned int buf;
>>
>> @@ -67,7 +62,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
>> } else {
>> int log = 2 * av_log2(buf) - 31;
>> if (log < 0) {
>> -av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> +if (emit_error_msg)
>> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> return AVERROR_INVALIDDATA;
>> }
>> buf >>= log;
>> @@ -92,7 +88,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
>> LAST_SKIP_BITS(re, gb, 32 - log);
>> CLOSE_READER(re, gb);
>> if (log < 7) {
>> -av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> +if (emit_error_msg)
>> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>> return AVERROR_INVALIDDATA;
>> }
>> buf >>= log;
>> @@ -103,6 +100,25 @@ static inline int get_ue_golomb(GetBitContext *gb)
>> #endif
>> }
>>
>> +/**
>> + * Read an unsigned Exp-Golomb code in the range 0 to 8190.
>> + *
>> + * @returns the read value or a negative error code.
>> + */
>> +static inline int get_ue_golomb(GetBitContext *gb)
>> +{
>> +return get_ue_golomb_internal(gb, 1);
>> +}
>> +
>> +/**
>> + * Variant of get_ue_golomb that does not emit an error message
>> + * if the number is outside the permissible range.
>> + */
>> +static inline int get_ue_golomb2(GetBitContext *gb)
>
> Seeing there's precedent for functions where the number suffix relates
> to the range of values they can parse, i think using a 2 here could be
> confusing at first glance. I suggest to use something else.
>
> Is it worth keeping the error messages at all, for that matter? They
> don't even use a proper logging context, so the printed output is ugly
> and unidentifiable by itself.
>
I could live very well without the error messages. If there are no
objections, I will just remove them (and not add them in the cached
codepath).
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/7] avcodec/golomb: Make emitting error message in get_ue_golomb optional
James Almer (12020-07-14): > Is it worth keeping the error messages at all, for that matter? They > don't even use a proper logging context, so the printed output is ugly > and unidentifiable by itself. Good point. We should probably try to eliminate all av_log(NULL) uses from the libraries, in fact. There are only ~300 of them, after all, that can be done. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 2/7] avcodec/golomb: Make emitting error message in get_ue_golomb optional
On 7/14/2020 12:34 PM, Andreas Rheinhardt wrote:
> This is designed for scenarios where the caller already checks that the
> returned value is within a certain allowed range and returns an error
> message if not.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavcodec/golomb.h | 32
> 1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
> index 5bfcfe085f..63069f63e5 100644
> --- a/libavcodec/golomb.h
> +++ b/libavcodec/golomb.h
> @@ -47,12 +47,7 @@ extern const uint8_t
> ff_interleaved_ue_golomb_vlc_code[256];
> extern const int8_t ff_interleaved_se_golomb_vlc_code[256];
> extern const uint8_t ff_interleaved_dirac_golomb_vlc_code[256];
>
> -/**
> - * Read an unsigned Exp-Golomb code in the range 0 to 8190.
> - *
> - * @returns the read value or a negative error code.
> - */
> -static inline int get_ue_golomb(GetBitContext *gb)
> +static inline int get_ue_golomb_internal(GetBitContext *gb, int
> emit_error_msg)
> {
> unsigned int buf;
>
> @@ -67,7 +62,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
> } else {
> int log = 2 * av_log2(buf) - 31;
> if (log < 0) {
> -av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> +if (emit_error_msg)
> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> return AVERROR_INVALIDDATA;
> }
> buf >>= log;
> @@ -92,7 +88,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
> LAST_SKIP_BITS(re, gb, 32 - log);
> CLOSE_READER(re, gb);
> if (log < 7) {
> -av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> +if (emit_error_msg)
> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
> return AVERROR_INVALIDDATA;
> }
> buf >>= log;
> @@ -103,6 +100,25 @@ static inline int get_ue_golomb(GetBitContext *gb)
> #endif
> }
>
> +/**
> + * Read an unsigned Exp-Golomb code in the range 0 to 8190.
> + *
> + * @returns the read value or a negative error code.
> + */
> +static inline int get_ue_golomb(GetBitContext *gb)
> +{
> +return get_ue_golomb_internal(gb, 1);
> +}
> +
> +/**
> + * Variant of get_ue_golomb that does not emit an error message
> + * if the number is outside the permissible range.
> + */
> +static inline int get_ue_golomb2(GetBitContext *gb)
Seeing there's precedent for functions where the number suffix relates
to the range of values they can parse, i think using a 2 here could be
confusing at first glance. I suggest to use something else.
Is it worth keeping the error messages at all, for that matter? They
don't even use a proper logging context, so the printed output is ugly
and unidentifiable by itself.
> +{
> +return get_ue_golomb_internal(gb, 0);
> +}
> +
> /**
> * Read an unsigned Exp-Golomb code in the range 0 to UINT32_MAX-1.
> */
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] x86/yuv2rgb: fix crashes when storing data on unaligned buffers
Am Mo., 13. Juli 2020 um 15:19 Uhr schrieb James Almer : > > On 7/13/2020 4:02 AM, Carl Eugen Hoyos wrote: > > > > > >> Am 13.07.2020 um 02:33 schrieb James Almer : > >> > >> Regression since fc6a5883d6af8cae0e96af84dda0ad74b360a084 on SSSE3 enabled > >> CPUs. > > > > FFmpeg does not require aligned output buffers on x86_64? > > There's no alignment requirement mentioned anywhere in swscale.h, which > explains why every other asm function also uses unaligned mov > instructions when handling src and dst buffers. Thank you both for the explanation, I misremembered. > >> Fixes ticket #8747 > > > > Another issue is described in this ticket that implies an overwrite: Can > > you reproduce? > > No, I haven't tried that one under valgrind. Given that I was unable to produce unaligned buffers for above issue, this part of the report looked more important to me. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 4/4] libavcodec/jpeg2000dec: Support for PPM marker
From: Gautam Ramakrishnan
This patch adds support for PPM marker for JPEG2000
decoder. It allows the samples p1_03.j2k and p1_05.j2k
to be decoded.
---
libavcodec/jpeg2000dec.c | 111 +++
1 file changed, 101 insertions(+), 10 deletions(-)
diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 5ea6fd0b9a..4f3a5af20e 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -71,6 +71,7 @@ typedef struct Jpeg2000POC {
typedef struct Jpeg2000TilePart {
uint8_t tile_index; // Tile index who refers the tile-part
const uint8_t *tp_end;
+GetByteContext header_tpg; // bit stream of header if PPM header
is used
GetByteContext tpg; // bit stream in tile-part
} Jpeg2000TilePart;
@@ -102,6 +103,13 @@ typedef struct Jpeg2000DecoderContext {
uint8_t cbps[4];// bits per sample in particular components
uint8_t sgnd[4];// if a component is signed
uint8_t properties[4];
+
+uint8_t has_ppm;
+uint8_t *packed_headers; // contains packed headers. Used only
along with PPM marker
+int packed_headers_size;
+GetByteContext packed_headers_stream;
+uint8_t in_tile_headers;
+
int cdx[4], cdy[4];
int precision;
int ncomponents;
@@ -928,6 +936,31 @@ static int get_plt(Jpeg2000DecoderContext *s, int n)
return 0;
}
+static int get_ppm(Jpeg2000DecoderContext *s, int n)
+{
+void *new;
+
+if (n < 3) {
+av_log(s->avctx, AV_LOG_ERROR, "Invalid length for PPM data.\n");
+return AVERROR_INVALIDDATA;
+}
+s->has_ppm = 1;
+bytestream2_get_byte(>g); //Zppm is skipped and not used
+new = av_realloc(s->packed_headers,
+ s->packed_headers_size + n - 3);
+if (new) {
+s->packed_headers = new;
+} else
+return AVERROR(ENOMEM);
+memset(>packed_headers_stream, 0, sizeof(s->packed_headers_stream));
+memcpy(s->packed_headers + s->packed_headers_size,
+ s->g.buffer, n - 3);
+s->packed_headers_size += n - 3;
+bytestream2_skip(>g, n - 3);
+
+return 0;
+}
+
static int get_ppt(Jpeg2000DecoderContext *s, int n)
{
Jpeg2000Tile *tile;
@@ -1039,8 +1072,19 @@ static int getlblockinc(Jpeg2000DecoderContext *s)
return res;
}
-static inline void select_stream(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile,
+static inline void select_header(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile,
int *tp_index)
+{
+s->g = tile->tile_part[*tp_index].header_tpg;
+if (bytestream2_get_bytes_left(>g) == 0 && s->bit_index == 8) {
+if (*tp_index < FF_ARRAY_ELEMS(tile->tile_part) - 1) {
+s->g = tile->tile_part[++(*tp_index)].tpg;
+}
+}
+}
+
+static inline void select_stream(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile,
+ int *tp_index, Jpeg2000CodingStyle *codsty)
{
s->g = tile->tile_part[*tp_index].tpg;
if (bytestream2_get_bytes_left(>g) == 0 && s->bit_index == 8) {
@@ -1048,8 +1092,12 @@ static inline void select_stream(Jpeg2000DecoderContext
*s, Jpeg2000Tile *tile,
s->g = tile->tile_part[++(*tp_index)].tpg;
}
}
-if (bytestream2_peek_be32(>g) == JPEG2000_SOP_FIXED_BYTES)
-bytestream2_skip(>g, JPEG2000_SOP_BYTE_LENGTH);
+if (codsty->csty & JPEG2000_CSTY_SOP) {
+if (bytestream2_peek_be32(>g) == JPEG2000_SOP_FIXED_BYTES)
+bytestream2_skip(>g, JPEG2000_SOP_BYTE_LENGTH);
+else
+av_log(s->avctx, AV_LOG_ERROR, "SOP marker not found. instead
%X\n", bytestream2_peek_be32(>g));
+}
}
static int jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile
*tile, int *tp_index,
@@ -1064,10 +1112,12 @@ static int
jpeg2000_decode_packet(Jpeg2000DecoderContext *s, Jpeg2000Tile *tile,
return 0;
rlevel->band[0].prec[precno].decoded_layers = layno + 1;
// Select stream to read from
-if (tile->has_ppt)
+if (s->has_ppm)
+select_header(s, tile, tp_index);
+else if (tile->has_ppt)
s->g = tile->packed_headers_stream;
else
-select_stream(s, tile, tp_index);
+select_stream(s, tile, tp_index, codsty);
if (!(ret = get_bits(s, 1))) {
jpeg2000_flush(s);
@@ -1178,9 +1228,12 @@ static int jpeg2000_decode_packet(Jpeg2000DecoderContext
*s, Jpeg2000Tile *tile,
}
// Save state of stream
-if (tile->has_ppt) {
+if (s->has_ppm) {
+tile->tile_part[*tp_index].header_tpg = s->g;
+select_stream(s, tile, tp_index, codsty);
+} else if (tile->has_ppt) {
tile->packed_headers_stream = s->g;
-select_stream(s, tile, tp_index);
+select_stream(s, tile, tp_index, codsty);
}
for (bandno = 0; bandno < rlevel->nbands; bandno++) {
Jpeg2000Band *band
[FFmpeg-devel] [PATCH v2 1/4] libavcodec/jpeg2000dec : Prevent overriding SOP marker bit
From: Gautam Ramakrishnan
Currently, the COC marker overrides the SOP marker bit.
However, only the COD marker may set this value. This
patch fixes this bug.
---
libavcodec/jpeg2000dec.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 18a933077e..48ca1c37a5 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -588,7 +588,7 @@ static int get_coc(Jpeg2000DecoderContext *s,
Jpeg2000CodingStyle *c,
uint8_t *properties)
{
int compno, ret;
-uint8_t has_eph;
+uint8_t has_eph, has_sop;
if (bytestream2_get_bytes_left(>g) < 2) {
av_log(s->avctx, AV_LOG_ERROR, "Insufficient space for COC\n");
@@ -606,8 +606,10 @@ static int get_coc(Jpeg2000DecoderContext *s,
Jpeg2000CodingStyle *c,
c += compno;
has_eph = c->csty & JPEG2000_CSTY_EPH;
+has_sop = c->csty & JPEG2000_CSTY_SOP;
c->csty = bytestream2_get_byteu(>g);
c->csty |= has_eph; //do not override eph present bits from COD
+c->csty |= has_sop; //do not override sop present bits from COD
if ((ret = get_cox(s, c)) < 0)
return ret;
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 2/4] libavcodec/jpeg2000 Fix PCRL Progression Order check
From: Gautam Ramakrishnan
The PCRL progression checks were incomplete. This patch
modifes completes the check. Tested on p1_05.j2k.
---
libavcodec/jpeg2000dec.c | 23 +++
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c
index 48ca1c37a5..adaac192e9 100644
--- a/libavcodec/jpeg2000dec.c
+++ b/libavcodec/jpeg2000dec.c
@@ -1465,23 +1465,30 @@ static int
jpeg2000_decode_packets_po_iteration(Jpeg2000DecoderContext *s, Jpeg2
Jpeg2000Component *comp = tile->comp + compno;
Jpeg2000CodingStyle *codsty = tile->codsty + compno;
Jpeg2000QuantStyle *qntsty = tile->qntsty + compno;
-int xc = x / s->cdx[compno];
-int yc = y / s->cdy[compno];
+
+if (!s->cdx[compno] || !s->cdy[compno])
+return AVERROR_INVALIDDATA;
for (reslevelno = RSpoc; reslevelno <
FFMIN(codsty->nreslevels, REpoc); reslevelno++) {
unsigned prcx, prcy;
uint8_t reducedresno = codsty->nreslevels - 1
-reslevelno; // ==> N_L - r
Jpeg2000ResLevel *rlevel = comp->reslevel + reslevelno;
+int trx0, try0;
-if (yc % (1LL << (rlevel->log2_prec_height +
reducedresno)) && y != tile->coord[1][0]) //FIXME this is a subset of the check
-continue;
+trx0 = ff_jpeg2000_ceildiv(tile->coord[0][0],
s->cdx[compno] << reducedresno);
+try0 = ff_jpeg2000_ceildiv(tile->coord[1][0],
s->cdy[compno] << reducedresno);
-if (xc % (1LL << (rlevel->log2_prec_width +
reducedresno)) && x != tile->coord[0][0]) //FIXME this is a subset of the check
-continue;
+if (!(y % ((uint64_t)s->cdy[compno] <<
(rlevel->log2_prec_height + reducedresno)) == 0 ||
+ (y == tile->coord[1][0] && (try0 << reducedresno)
% (1U << (reducedresno + rlevel->log2_prec_height)
+ continue;
+
+if (!(x % ((uint64_t)s->cdx[compno] <<
(rlevel->log2_prec_width + reducedresno)) == 0 ||
+ (x == tile->coord[0][0] && (trx0 << reducedresno)
% (1U << (reducedresno + rlevel->log2_prec_width)
+ continue;
// check if a precinct exists
-prcx = ff_jpeg2000_ceildivpow2(xc, reducedresno) >>
rlevel->log2_prec_width;
-prcy = ff_jpeg2000_ceildivpow2(yc, reducedresno) >>
rlevel->log2_prec_height;
+prcx = ff_jpeg2000_ceildiv(x, s->cdx[compno] <<
reducedresno) >> rlevel->log2_prec_width;
+prcy = ff_jpeg2000_ceildiv(y, s->cdy[compno] <<
reducedresno) >> rlevel->log2_prec_height;
prcx -= ff_jpeg2000_ceildivpow2(comp->coord_o[0][0],
reducedresno) >> rlevel->log2_prec_width;
prcy -= ff_jpeg2000_ceildivpow2(comp->coord_o[1][0],
reducedresno) >> rlevel->log2_prec_height;
--
2.17.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2 3/4] libavcodec/jpeg2000 Fix RPCL Progression order check
From: Gautam Ramakrishnan The RPCL progression order check was incomplete. This patch completes the check. Tested on p1_07.j2k. --- libavcodec/jpeg2000dec.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/libavcodec/jpeg2000dec.c b/libavcodec/jpeg2000dec.c index adaac192e9..5ea6fd0b9a 100644 --- a/libavcodec/jpeg2000dec.c +++ b/libavcodec/jpeg2000dec.c @@ -1395,22 +1395,28 @@ static int jpeg2000_decode_packets_po_iteration(Jpeg2000DecoderContext *s, Jpeg2 uint8_t reducedresno = codsty->nreslevels - 1 -reslevelno; // ==> N_L - r Jpeg2000ResLevel *rlevel = comp->reslevel + reslevelno; unsigned prcx, prcy; +int trx0, try0; -int xc = x / s->cdx[compno]; -int yc = y / s->cdy[compno]; +if (!s->cdx[compno] || !s->cdy[compno]) +return AVERROR_INVALIDDATA; + +trx0 = ff_jpeg2000_ceildiv(tile->coord[0][0], s->cdx[compno] << reducedresno); +try0 = ff_jpeg2000_ceildiv(tile->coord[1][0], s->cdy[compno] << reducedresno); if (reslevelno >= codsty->nreslevels) continue; -if (yc % (1LL << (rlevel->log2_prec_height + reducedresno)) && y != tile->coord[1][0]) //FIXME this is a subset of the check +if (!(y % ((uint64_t)s->cdy[compno] << (rlevel->log2_prec_height + reducedresno)) == 0 || + (y == tile->coord[1][0] && (try0 << reducedresno) % (1U << (reducedresno + rlevel->log2_prec_height) continue; -if (xc % (1LL << (rlevel->log2_prec_width + reducedresno)) && x != tile->coord[0][0]) //FIXME this is a subset of the check +if (!(x % ((uint64_t)s->cdx[compno] << (rlevel->log2_prec_width + reducedresno)) == 0 || + (x == tile->coord[0][0] && (trx0 << reducedresno) % (1U << (reducedresno + rlevel->log2_prec_width) continue; // check if a precinct exists -prcx = ff_jpeg2000_ceildivpow2(xc, reducedresno) >> rlevel->log2_prec_width; -prcy = ff_jpeg2000_ceildivpow2(yc, reducedresno) >> rlevel->log2_prec_height; +prcx = ff_jpeg2000_ceildiv(x, s->cdx[compno] << reducedresno) >> rlevel->log2_prec_width; +prcy = ff_jpeg2000_ceildiv(y, s->cdy[compno] << reducedresno) >> rlevel->log2_prec_height; prcx -= ff_jpeg2000_ceildivpow2(comp->coord_o[0][0], reducedresno) >> rlevel->log2_prec_width; prcy -= ff_jpeg2000_ceildivpow2(comp->coord_o[1][0], reducedresno) >> rlevel->log2_prec_height; -- 2.17.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avcodec/golomb: Prevent shift by negative number
Michael Niedermayer:
> On Mon, Jul 13, 2020 at 09:04:30PM +0200, Tomas Härdin wrote:
>> fre 2020-07-10 klockan 15:48 +0200 skrev Andreas Rheinhardt:
>>> This happened in get_ue_golomb() if the cached bitstream reader was
>>> in
>>> use, because there was no check to handle the case of the read value
>>> not being in the range 0..8190.
>>>
>>> Signed-off-by: Andreas Rheinhardt
>>> ---
>>> libavcodec/golomb.h | 4
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
>>> index 7fd46a91bd..5bfcfe085f 100644
>>> --- a/libavcodec/golomb.h
>>> +++ b/libavcodec/golomb.h
>>> @@ -66,6 +66,10 @@ static inline int get_ue_golomb(GetBitContext *gb)
>>> return ff_ue_golomb_vlc_code[buf];
>>> } else {
>>> int log = 2 * av_log2(buf) - 31;
>>> +if (log < 0) {
>>> +av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
>>> +return AVERROR_INVALIDDATA;
>>> +}
>>
>> How come invalid codes can even be present like this? Seems
>> inefficient. Or is the decoder wrong? Maybe Michael wants to chime in
>> here, since he wrote the original implementation.
>
> The codepath of the non cached one does a check too. It should be
> done consistently.
Does this imply that you want me to error out for log < 7 although the
cached bitstream reader is able to read the number even when log is in
the range of 0..6? (The reason for this is that one only gets 25 valid
bits in the noncached version.)
> If the check causes a meassurable speedloss, then it can be implemented
> without a check. For example by using asm for the shift operation, or maybe
> compilers have some nicer function doing a never undefined shift. If such
> function does not exist, its something that maybe someone should add to
> gcc & clang because other projects could benefit from it too in similar
> situations.
22e960ad478e568f4094971a58c6ad8f549c0180 (which enabled the check and
added the av_log() in the noncached branch) claims no measurable speedloss.
I have not found any such compiler builtin function for this purpose;
neither GCC nor Clang have one.
Notice that if we had such an always defined shift, we could use it to
make put_bits() write 0-32 bits at once if this always defined shift had
the property that 0U << 32 equals 0.
- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 6/7] avcodec/h264*: Omit potentially wrong values from log messages
get_ue_golomb_30() and get_ue_golomb() (as well as
get_ue_golomb2()) can only parse values within a certain range
correctly; if the parsed value is not within said range, the latter two
functions return AVERROR_INVALIDDATA, while the former returns something
outside of said range (currently 32 for a 0-30 range). So the return
values are good enough to determine whether an exp golomb code in the
desired range has been encountered, but they are not necessarily
correct. Therefore they should not be used in error messages stating
that a certain value (the return value of these functions) is
out-of-range; instead just state the correct range and that the parsed
value is not in said range.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cavsdec.c | 2 +-
libavcodec/h264_cavlc.c | 14 +++---
libavcodec/h264_parse.c | 4 ++--
libavcodec/h264_parser.c | 2 +-
libavcodec/h264_ps.c | 26 ++
libavcodec/h264_slice.c | 2 +-
6 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 1c8585f5af..0aea9757b7 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -615,7 +615,7 @@ static inline int decode_residual_inter(AVSContext *h)
/* get coded block pattern */
int cbp = get_ue_golomb(>gb);
if (cbp > 63U) {
-av_log(h->avctx, AV_LOG_ERROR, "illegal inter cbp %d\n", cbp);
+av_log(h->avctx, AV_LOG_ERROR, "Inter cbp not in 0-63 range\n");
return AVERROR_INVALIDDATA;
}
h->cbp = cbp_tab[cbp][1];
diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c
index 51091abbe1..c4b14d2578 100644
--- a/libavcodec/h264_cavlc.c
+++ b/libavcodec/h264_cavlc.c
@@ -762,7 +762,7 @@ int ff_h264_decode_mb_cavlc(const H264Context *h,
H264SliceContext *sl)
mb_type--;
decode_intra_mb:
if(mb_type > 25){
-av_log(h->avctx, AV_LOG_ERROR, "mb_type %d in %c slice too large
at %d %d\n", mb_type, av_get_picture_type_char(sl->slice_type), sl->mb_x,
sl->mb_y);
+av_log(h->avctx, AV_LOG_ERROR, "mb_type in %c slice too large at
%d %d\n", av_get_picture_type_char(sl->slice_type), sl->mb_x, sl->mb_y);
return -1;
}
partition_count=0;
@@ -891,7 +891,7 @@ decode_intra_mb:
}else{
tmp = get_ue_golomb2(>gb);
if(tmp>=ref_count){
-av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", tmp);
+av_log(h->avctx, AV_LOG_ERROR, "ref overflow\n");
return -1;
}
}
@@ -967,7 +967,7 @@ decode_intra_mb:
}else{
val = get_ue_golomb2(>gb);
if (val >= rc) {
-av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
+av_log(h->avctx, AV_LOG_ERROR, "ref
overflow\n");
return -1;
}
}
@@ -998,7 +998,7 @@ decode_intra_mb:
}else{
val = get_ue_golomb2(>gb);
if (val >= rc) {
-av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
+av_log(h->avctx, AV_LOG_ERROR, "ref
overflow\n");
return -1;
}
}
@@ -1036,7 +1036,7 @@ decode_intra_mb:
}else{
val = get_ue_golomb2(>gb);
if (val >= rc) {
-av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
+av_log(h->avctx, AV_LOG_ERROR, "ref
overflow\n");
return -1;
}
}
@@ -1071,7 +1071,7 @@ decode_intra_mb:
if(decode_chroma){
if(cbp > 47){
-av_log(h->avctx, AV_LOG_ERROR, "cbp too large (%u) at %d
%d\n", cbp, sl->mb_x, sl->mb_y);
+av_log(h->avctx, AV_LOG_ERROR, "cbp >%d at %d %d\n", 47,
sl->mb_x, sl->mb_y);
return -1;
}
if (IS_INTRA4x4(mb_type))
@@ -1080,7 +1080,7 @@ decode_intra_mb:
cbp = ff_h264_golomb_to_inter_cbp[cbp];
}else{
if(cbp > 15){
-av_log(h->avctx, AV_LOG_ERROR, "cbp too large (%u) at %d
%d\n", cbp, sl->mb_x, sl->mb_y);
+av_log(h->avctx, AV_LOG_ERROR, "cbp >%d at %d %d\n", 15,
sl->mb_x, sl->mb_y);
return -1;
}
if(IS_INTRA4x4(mb_type)) cbp= golomb_to_intra4x4_cbp_gray[cbp];
diff --git a/libavcodec/h264_parse.c
[FFmpeg-devel] [PATCH 7/7] avcodec/h264*: Use get_ue_golomb2() where appropriate
The difference of this function and get_ue_golomb() is that the latter
emits an error message upon encountering an exp golomb code outside the
range of 0..8190. But this error message is unnecessary in cases where
there is a dedicate error message in case of range error (these error
messages also use a correct logcontext and not NULL). So replace
get_ue_golomb() by get_ue_golomb2() everywhere where there is a
dedicated error message in case of errors.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cavsdec.c | 6 +++---
libavcodec/h264_cavlc.c | 4 ++--
libavcodec/h264_parse.c | 4 ++--
libavcodec/h264_parser.c | 2 +-
libavcodec/h264_ps.c | 8
libavcodec/h264_slice.c | 2 +-
6 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index 0aea9757b7..8da57ec80a 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -502,7 +502,7 @@ static inline void mv_pred_sym(AVSContext *h, cavs_vector
*src,
/** kth-order exponential golomb code */
static inline int get_ue_code(GetBitContext *gb, int order)
{
-unsigned ret = get_ue_golomb(gb);
+unsigned ret = get_ue_golomb2(gb);
if (ret >= ((1U<<31)>>order)) {
av_log(NULL, AV_LOG_ERROR, "get_ue_code: value too larger\n");
return AVERROR_INVALIDDATA;
@@ -613,7 +613,7 @@ static inline int decode_residual_inter(AVSContext *h)
int block;
/* get coded block pattern */
-int cbp = get_ue_golomb(>gb);
+int cbp = get_ue_golomb2(>gb);
if (cbp > 63U) {
av_log(h->avctx, AV_LOG_ERROR, "Inter cbp not in 0-63 range\n");
return AVERROR_INVALIDDATA;
@@ -685,7 +685,7 @@ static int decode_mb_i(AVSContext *h, int cbp_code)
/* get coded block pattern */
if (h->cur.f->pict_type == AV_PICTURE_TYPE_I)
-cbp_code = get_ue_golomb(gb);
+cbp_code = get_ue_golomb2(gb);
if (cbp_code > 63U) {
av_log(h->avctx, AV_LOG_ERROR, "illegal intra cbp\n");
return AVERROR_INVALIDDATA;
diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c
index c4b14d2578..3dcb82c71c 100644
--- a/libavcodec/h264_cavlc.c
+++ b/libavcodec/h264_cavlc.c
@@ -739,7 +739,7 @@ int ff_h264_decode_mb_cavlc(const H264Context *h,
H264SliceContext *sl)
sl->prev_mb_skipped = 0;
-mb_type= get_ue_golomb(>gb);
+mb_type = get_ue_golomb2(>gb);
if (sl->slice_type_nos == AV_PICTURE_TYPE_B) {
if(mb_type < 23){
partition_count = ff_h264_b_mb_type_info[mb_type].partition_count;
@@ -1067,7 +1067,7 @@ decode_intra_mb:
write_back_motion(h, sl, mb_type);
if(!IS_INTRA16x16(mb_type)){
-cbp= get_ue_golomb(>gb);
+cbp = get_ue_golomb2(>gb);
if(decode_chroma){
if(cbp > 47){
diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index fd9b824756..11ae20a32a 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -234,9 +234,9 @@ int ff_h264_parse_ref_count(int *plist_count, int
ref_count[2],
num_ref_idx_active_override_flag = get_bits1(gb);
if (num_ref_idx_active_override_flag) {
-ref_count[0] = get_ue_golomb(gb) + 1;
+ref_count[0] = get_ue_golomb2(gb) + 1;
if (slice_type_nos == AV_PICTURE_TYPE_B) {
-ref_count[1] = get_ue_golomb(gb) + 1;
+ref_count[1] = get_ue_golomb2(gb) + 1;
} else
// full range is spec-ok in this case, even for frames
ref_count[1] = 1;
diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c
index 3b62567245..d5bf80a89a 100644
--- a/libavcodec/h264_parser.c
+++ b/libavcodec/h264_parser.c
@@ -348,7 +348,7 @@ static inline int parse_nal_units(AVCodecParserContext *s,
/* key frame, since recovery_frame_cnt is set */
s->key_frame = 1;
}
-pps_id = get_ue_golomb();
+pps_id = get_ue_golomb2();
if (pps_id >= MAX_PPS_COUNT) {
av_log(avctx, AV_LOG_ERROR,
"pps_id not in 0-255 range\n");
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index 18e5d9578f..9f0d07810b 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -459,7 +459,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb,
AVCodecContext *avctx,
goto fail;
}
-sps->poc_cycle_length = get_ue_golomb(gb);
+sps->poc_cycle_length = get_ue_golomb2(gb);
if ((unsigned)sps->poc_cycle_length >=
FF_ARRAY_ELEMS(sps->offset_for_ref_frame)) {
@@ -750,7 +750,7 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb,
AVCodecContext *avct
{
AVBufferRef *pps_buf;
const SPS *sps;
-unsigned int pps_id = get_ue_golomb(gb);
+unsigned int pps_id = get_ue_golomb2(gb);
PPS *pps;
int qp_bd_offset;
int bits_left;
@@ -821,8 +821,8 @@ int
[FFmpeg-devel] [PATCH 5/7] avcodec/cavsdec, h264*, hevc_parser: Use get_ue_golomb_30 where possible
instead of get_ue_golomb(). The difference between the two is that the
latter also has to take into account the case in which the read code is
more than 9 bits (four preceding zeroes + at most five value bits) long.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/cavsdec.c | 4 ++--
libavcodec/h264_parse.c | 4 ++--
libavcodec/h264_ps.c | 22 +++---
libavcodec/hevc_parser.c | 2 +-
4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/libavcodec/cavsdec.c b/libavcodec/cavsdec.c
index aaed807196..1c8585f5af 100644
--- a/libavcodec/cavsdec.c
+++ b/libavcodec/cavsdec.c
@@ -676,8 +676,8 @@ static int decode_mb_i(AVSContext *h, int cbp_code)
}
h->pred_mode_Y[pos] = predpred;
}
-pred_mode_uv = get_ue_golomb(gb);
-if (pred_mode_uv > 6) {
+pred_mode_uv = get_ue_golomb_30(gb);
+if (pred_mode_uv > 6U) {
av_log(h->avctx, AV_LOG_ERROR, "illegal intra chroma pred mode\n");
return AVERROR_INVALIDDATA;
}
diff --git a/libavcodec/h264_parse.c b/libavcodec/h264_parse.c
index 352ffea948..53e644da66 100644
--- a/libavcodec/h264_parse.c
+++ b/libavcodec/h264_parse.c
@@ -35,7 +35,7 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS
*sps,
pwt->use_weight = 0;
pwt->use_weight_chroma = 0;
-pwt->luma_log2_weight_denom = get_ue_golomb(gb);
+pwt->luma_log2_weight_denom = get_ue_golomb_30(gb);
if (pwt->luma_log2_weight_denom > 7U) {
av_log(logctx, AV_LOG_ERROR, "luma_log2_weight_denom %d is out of
range\n", pwt->luma_log2_weight_denom);
pwt->luma_log2_weight_denom = 0;
@@ -43,7 +43,7 @@ int ff_h264_pred_weight_table(GetBitContext *gb, const SPS
*sps,
luma_def = 1 << pwt->luma_log2_weight_denom;
if (sps->chroma_format_idc) {
-pwt->chroma_log2_weight_denom = get_ue_golomb(gb);
+pwt->chroma_log2_weight_denom = get_ue_golomb_30(gb);
if (pwt->chroma_log2_weight_denom > 7U) {
av_log(logctx, AV_LOG_ERROR, "chroma_log2_weight_denom %d is out
of range\n", pwt->chroma_log2_weight_denom);
pwt->chroma_log2_weight_denom = 0;
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
index 13823b8dc0..5ad311e52e 100644
--- a/libavcodec/h264_ps.c
+++ b/libavcodec/h264_ps.c
@@ -181,8 +181,8 @@ static inline int decode_vui_parameters(GetBitContext *gb,
void *logctx,
/* chroma_location_info_present_flag */
if (get_bits1(gb)) {
/* chroma_sample_location_type_top_field */
-sps->chroma_location = get_ue_golomb(gb) + 1;
-get_ue_golomb(gb); /* chroma_sample_location_type_bottom_field */
+sps->chroma_location = get_ue_golomb_30(gb) + 1;
+get_ue_golomb_30(gb); /* chroma_sample_location_type_bottom_field */
} else
sps->chroma_location = AVCHROMA_LOC_LEFT;
@@ -224,11 +224,11 @@ static inline int decode_vui_parameters(GetBitContext
*gb, void *logctx,
sps->bitstream_restriction_flag = get_bits1(gb);
if (sps->bitstream_restriction_flag) {
get_bits1(gb); /* motion_vectors_over_pic_boundaries_flag */
-get_ue_golomb(gb); /* max_bytes_per_pic_denom */
-get_ue_golomb(gb); /* max_bits_per_mb_denom */
-get_ue_golomb(gb); /* log2_max_mv_length_horizontal */
-get_ue_golomb(gb); /* log2_max_mv_length_vertical */
-sps->num_reorder_frames = get_ue_golomb(gb);
+get_ue_golomb_30(gb); /* max_bytes_per_pic_denom */
+get_ue_golomb_30(gb); /* max_bits_per_mb_denom */
+get_ue_golomb_30(gb); /* log2_max_mv_length_horizontal */
+get_ue_golomb_30(gb); /* log2_max_mv_length_vertical */
+sps->num_reorder_frames = get_ue_golomb_30(gb);
get_ue_golomb(gb); /*max_dec_frame_buffering*/
if (get_bits_left(gb) < 0) {
@@ -403,8 +403,8 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb,
AVCodecContext *avctx,
goto fail;
}
}
-sps->bit_depth_luma = get_ue_golomb(gb) + 8;
-sps->bit_depth_chroma = get_ue_golomb(gb) + 8;
+sps->bit_depth_luma = get_ue_golomb_30(gb) + 8;
+sps->bit_depth_chroma = get_ue_golomb_30(gb) + 8;
if (sps->bit_depth_chroma != sps->bit_depth_luma) {
avpriv_request_sample(avctx,
"Different chroma and luma bit depth");
@@ -428,7 +428,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb,
AVCodecContext *avctx,
sps->bit_depth_chroma = 8;
}
-log2_max_frame_num_minus4 = get_ue_golomb(gb);
+log2_max_frame_num_minus4 = get_ue_golomb_30(gb);
if (log2_max_frame_num_minus4 < MIN_LOG2_MAX_FRAME_NUM - 4 ||
log2_max_frame_num_minus4 > MAX_LOG2_MAX_FRAME_NUM - 4) {
av_log(avctx, AV_LOG_ERROR,
@@ -441,7 +441,7 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb,
AVCodecContext *avctx,
sps->poc_type = get_ue_golomb_30(gb);
if (sps->poc_type == 0) { // FIXME
[FFmpeg-devel] [PATCH 3/7] avcodec/golomb, h264*: Fix get_ue_golomb_31()
get_ue_golomb_31() uses a LUT of 512 entries; therefore it can be used
to parse exp-golomb codes of length <= 9, i.e. those codes with at most
four leading bits that have five effective bits; this implies a range of
0..30 and not 31. In particular, this function must not be used to parse
e.g. the H.264 SPS id.
This commit renames the function to get_ue_golomb_30() and uses it
whereever possible. In places where it is not possible, it has been
replaced by get_ue_golomb2(). Given that the returned value of said
function can be a negative error code, it is no longer included in
av_log() statements (the parsed value can actually also be wrong when
using get_ue_golomb30()).
Signed-off-by: Andreas Rheinhardt
---
I have only used get_ue_golomb_30() where I am certain that its range is
sufficient; this unfortunately excludes certain cases in h264_cavlc,
because for field based content, the number of refs can be in the range
of 0..31.
libavcodec/golomb.h | 6 +++---
libavcodec/h264_cavlc.c | 14 +++---
libavcodec/h264_parser.c | 8
libavcodec/h264_ps.c | 18 +-
libavcodec/h264_refs.c | 6 +++---
libavcodec/h264_sei.c| 8
libavcodec/h264_slice.c | 6 +++---
libavformat/mxfenc.c | 2 +-
8 files changed, 34 insertions(+), 34 deletions(-)
diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 63069f63e5..172d9934e0 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -134,10 +134,10 @@ static inline unsigned get_ue_golomb_long(GetBitContext
*gb)
}
/**
- * read unsigned exp golomb code, constraint to a max of 31.
- * the return value is undefined if the stored value exceeds 31.
+ * Read unsigned exp golomb code, constraint to a max of 30.
+ * the return value is undefined if the stored value exceeds 30.
*/
-static inline int get_ue_golomb_31(GetBitContext *gb)
+static inline int get_ue_golomb_30(GetBitContext *gb)
{
unsigned int buf;
diff --git a/libavcodec/h264_cavlc.c b/libavcodec/h264_cavlc.c
index 6481992e58..51091abbe1 100644
--- a/libavcodec/h264_cavlc.c
+++ b/libavcodec/h264_cavlc.c
@@ -838,7 +838,7 @@ decode_intra_mb:
}
if(decode_chroma){
pred_mode= ff_h264_check_intra_pred_mode(h->avctx,
sl->top_samples_available,
-
sl->left_samples_available, get_ue_golomb_31(>gb), 1);
+
sl->left_samples_available, get_ue_golomb_30(>gb), 1);
if(pred_mode < 0)
return -1;
sl->chroma_pred_mode = pred_mode;
@@ -850,7 +850,7 @@ decode_intra_mb:
if (sl->slice_type_nos == AV_PICTURE_TYPE_B) {
for(i=0; i<4; i++){
-sl->sub_mb_type[i]= get_ue_golomb_31(>gb);
+sl->sub_mb_type[i] = get_ue_golomb_30(>gb);
if(sl->sub_mb_type[i] >=13){
av_log(h->avctx, AV_LOG_ERROR, "B sub_mb_type %u out of
range at %d %d\n", sl->sub_mb_type[i], sl->mb_x, sl->mb_y);
return -1;
@@ -868,7 +868,7 @@ decode_intra_mb:
}else{
av_assert2(sl->slice_type_nos == AV_PICTURE_TYPE_P); //FIXME SP
correct ?
for(i=0; i<4; i++){
-sl->sub_mb_type[i]= get_ue_golomb_31(>gb);
+sl->sub_mb_type[i] = get_ue_golomb_30(>gb);
if(sl->sub_mb_type[i] >=4){
av_log(h->avctx, AV_LOG_ERROR, "P sub_mb_type %u out of
range at %d %d\n", sl->sub_mb_type[i], sl->mb_x, sl->mb_y);
return -1;
@@ -889,7 +889,7 @@ decode_intra_mb:
}else if(ref_count == 2){
tmp= get_bits1(>gb)^1;
}else{
-tmp= get_ue_golomb_31(>gb);
+tmp = get_ue_golomb2(>gb);
if(tmp>=ref_count){
av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", tmp);
return -1;
@@ -965,7 +965,7 @@ decode_intra_mb:
} else if (rc == 2) {
val= get_bits1(>gb)^1;
}else{
-val= get_ue_golomb_31(>gb);
+val = get_ue_golomb2(>gb);
if (val >= rc) {
av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
return -1;
@@ -996,7 +996,7 @@ decode_intra_mb:
} else if (rc == 2) {
val= get_bits1(>gb)^1;
}else{
-val= get_ue_golomb_31(>gb);
+val = get_ue_golomb2(>gb);
if (val >= rc) {
av_log(h->avctx, AV_LOG_ERROR, "ref %u
overflow\n", val);
return -1;
@@ -1034,7
[FFmpeg-devel] [PATCH 4/7] avcodec/golomb: Document return value of get_ue_golomb_30 on error
If the return value is undefined if the value of the encountered
exp golomb code was outside the 0..30 range, said function can't
be used anywhere where one wants to check for correctness. So document
that in this case the return value is outside the range 0..30 (it
currently always returns 32 in this case).
Signed-off-by: Andreas Rheinhardt
---
Btw: get_ue_golomb_30() could be used to skip exp golomb codes with
length <= 17 bits.
libavcodec/golomb.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 172d9934e0..8b69196d44 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -135,7 +135,8 @@ static inline unsigned get_ue_golomb_long(GetBitContext *gb)
/**
* Read unsigned exp golomb code, constraint to a max of 30.
- * the return value is undefined if the stored value exceeds 30.
+ * The return value is outside the range of 0..30 if no exp golomb code
+ * in that range was encountered.
*/
static inline int get_ue_golomb_30(GetBitContext *gb)
{
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH 2/7] avcodec/golomb: Make emitting error message in get_ue_golomb optional
This is designed for scenarios where the caller already checks that the
returned value is within a certain allowed range and returns an error
message if not.
Signed-off-by: Andreas Rheinhardt
---
libavcodec/golomb.h | 32
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/libavcodec/golomb.h b/libavcodec/golomb.h
index 5bfcfe085f..63069f63e5 100644
--- a/libavcodec/golomb.h
+++ b/libavcodec/golomb.h
@@ -47,12 +47,7 @@ extern const uint8_t ff_interleaved_ue_golomb_vlc_code[256];
extern const int8_t ff_interleaved_se_golomb_vlc_code[256];
extern const uint8_t ff_interleaved_dirac_golomb_vlc_code[256];
-/**
- * Read an unsigned Exp-Golomb code in the range 0 to 8190.
- *
- * @returns the read value or a negative error code.
- */
-static inline int get_ue_golomb(GetBitContext *gb)
+static inline int get_ue_golomb_internal(GetBitContext *gb, int emit_error_msg)
{
unsigned int buf;
@@ -67,7 +62,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
} else {
int log = 2 * av_log2(buf) - 31;
if (log < 0) {
-av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
+if (emit_error_msg)
+av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
return AVERROR_INVALIDDATA;
}
buf >>= log;
@@ -92,7 +88,8 @@ static inline int get_ue_golomb(GetBitContext *gb)
LAST_SKIP_BITS(re, gb, 32 - log);
CLOSE_READER(re, gb);
if (log < 7) {
-av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
+if (emit_error_msg)
+av_log(NULL, AV_LOG_ERROR, "Invalid UE golomb code\n");
return AVERROR_INVALIDDATA;
}
buf >>= log;
@@ -103,6 +100,25 @@ static inline int get_ue_golomb(GetBitContext *gb)
#endif
}
+/**
+ * Read an unsigned Exp-Golomb code in the range 0 to 8190.
+ *
+ * @returns the read value or a negative error code.
+ */
+static inline int get_ue_golomb(GetBitContext *gb)
+{
+return get_ue_golomb_internal(gb, 1);
+}
+
+/**
+ * Variant of get_ue_golomb that does not emit an error message
+ * if the number is outside the permissible range.
+ */
+static inline int get_ue_golomb2(GetBitContext *gb)
+{
+return get_ue_golomb_internal(gb, 0);
+}
+
/**
* Read an unsigned Exp-Golomb code in the range 0 to UINT32_MAX-1.
*/
--
2.20.1
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/3] libavutil/imgutils: add utility to get plane sizes
On 7/14/2020 9:47 AM, Nicolas George wrote: > James Almer (12020-07-10): >> Because my opinion and tastes are not yours. I already said why *i* >> consider it ugly. It doesn't need to fit *your* conception of ugliness. > > If it is only a matter of taste, then it cannot count as an argument. > But tastes are frequently a proxy for minor factors. If you can express > the minor factors behind your tastes, they can count as arguments. > > Which is what I am trying to do about my tastes. > > One of these minor factors: there is frequently a "int ret" variable and > a "return ret" at the end. If the return value conflates size with the > error code, ret cannot be used, which means some kind of "if (size < 0) > ret = size;" need to happen, but would easily be forgotten, initially or > in refactoring. > >> int av_image_fill_plane_sizes(size_t sizes[4], enum AVPixelFormat >> pix_fmt, const ptrdiff_t linesizes[4]); > > So the question is now: is the total size useful enough to warrant an > extra return parameter or not? I have no advice on the question. Not for an extra parameter (Who doesn't love enterprise-style code full of function calls filled with NULL arguments, right?), but yes for using the otherwise undefined >0 scope of the return value, which is only possible if we keep the sizes as int like the rest of the module, and to be honest something i would very much like to do seeing almost every other existing function can't be ported to size_t/ptrdiff_t. A complete, consistent set of new functions would have to be introduced for that purpose. I'd like to convince Michael about it, since he suggested these types, but if i can't then this is the approach most consistent with existing size array filling functions. > > Regards, ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] libavfilter: avfilter_graph_parse2 is not preserving pad names
I wrote a code to mix two audio streams which is working. However,
somehow `avfilter_graph_parse2` is not preserving the pad names
For example, below is a filter spec
```
filter_spec =
"abuffer=time_base=1/48000:sample_rate=48000:sample_fmt=fltp:channel_layout=0x3[a0]
;
abuffer=time_base=1/48000:sample_rate=48000:sample_fmt=fltp:channel_layout=0x3
[a1];
[a0] [a1] amix=inputs=2, abuffersink";
```
Below is the code to parse and dump graph
```
if ((ret = avfilter_graph_parse2(filter_graph, filter_spec, ,
)) < 0) {
av_log(NULL, AV_LOG_ERROR, "avfilter_graph_parse_ptr
failed: %d\n", ret);
}
if ((ret = avfilter_graph_config(filter_graph, NULL)) < 0) {
av_log(NULL, AV_LOG_ERROR, "avfilter_graph_config failed\n");
}
char *g = avfilter_graph_dump(filter_graph, NULL);
if(g) {
av_log(NULL, AV_LOG_ERROR, "Graph: %s\n", g);
av_free(g);
}
```
and below is the graph dump.
```
Graph: +--+
| Parsed_abuffer_0 |default--[48000Hz fltp:stereo]--Parsed_amix_2:input0
|(abuffer) |
+--+
+--+
| Parsed_abuffer_1 |default--[48000Hz fltp:stereo]--Parsed_amix_2:input1
|(abuffer) |
+--+
+---+
Parsed_abuffer_0:default--[48000Hz fltp:stereo]--input0| Parsed_amix_2
|default--[48000Hz fltp:stereo]--Parsed_abuffersink_3:default
Parsed_abuffer_1:default--[48000Hz fltp:stereo]--input1|(amix) |
+---+
+--+
Parsed_amix_2:default--[48000Hz fltp:stereo]--default| Parsed_abuffersink_3 |
|(abuffersink) |
+--+
```
As you can see, all the pad names a0, a1, etc gone and they are
replaced by `default`, Parsed_abuffersink_3 etc.
Is there any way to preserve names?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] lavf/srt: fix build fail when used the libsrt 1.4.1
On Sun, Jul 12, 2020 at 22:44:46 +0800, myp...@gmail.com wrote: > Maybe I give an inaccurate description in the commit message, in fact, > libsrt 1.4.1 remove the SRTO_STRICTENC/SRTO_SMOOTHER option, it's will > lead to FFmpeg build fail when using libsrt version >= 1.4.1 I don't mind you description. Yet your code change references 1.4.1: SRT_VERSION_VALUE >= 0x010401 while I am trying to make the point that 1.4.0 also needs this change (or 1.3.0 in one case, which is already ensured by #if). Moritz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2 1/3] libavutil/imgutils: add utility to get plane sizes
James Almer (12020-07-10): > Because my opinion and tastes are not yours. I already said why *i* > consider it ugly. It doesn't need to fit *your* conception of ugliness. If it is only a matter of taste, then it cannot count as an argument. But tastes are frequently a proxy for minor factors. If you can express the minor factors behind your tastes, they can count as arguments. Which is what I am trying to do about my tastes. One of these minor factors: there is frequently a "int ret" variable and a "return ret" at the end. If the return value conflates size with the error code, ret cannot be used, which means some kind of "if (size < 0) ret = size;" need to happen, but would easily be forgotten, initially or in refactoring. > int av_image_fill_plane_sizes(size_t sizes[4], enum AVPixelFormat > pix_fmt, const ptrdiff_t linesizes[4]); So the question is now: is the total size useful enough to warrant an extra return parameter or not? I have no advice on the question. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] mpeg2: Renaming functions around init_uni_ac_vlc
On Mon, Jul 13, 2020 at 13:16:18 +0200, Jean-Baptiste Kempf wrote: > We need to export init_uni_ac_vlc to init_uni_ac_vlc and therefore need "to ff_init_uni_ac_vlc"? Moritz ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/riffenc: indicate storage of flipped RGB bitmaps
On 14-07-2020 05:14 pm, Andreas Rheinhardt wrote: Gyan Doshi: Some legacy applications such as AVI2MVE expect raw RGB bitmaps to be stored bottom-up, whereas our RIFF BITMAPINFOHEADER assumes they are always stored top-down and thus write a negative value for height. This can prevent reading of these files. Option flipped_raw_rgb added to AVI and Matroska muxers which will write positive value for height when enabled. Note that the user has to flip the bitmaps beforehand using other means such as the vflip filter. --- 1. The avi demuxer adds "BottomUp" to the extradata in case it demuxes bottom up data. The Matroska demuxer currently doesn't and so rawvideo stored in Matroska in vfw mode will always be treated as top-down. This needs to be fixed, too. Agreed. But that's a separate patch, 2. But why don't we add an option to the raw encoder so that it adds (or maybe switches?) "BottomUp" to the extradata to indicate that the output is bottom-up? That seems like a better place for me. (E.g. it would naturally allow to set the bottom-up/top-down property on a per-stream basis.) This "feature" is a quirk of Microsoft / RIFF containers (or their accommodation in other containers viz. VFW in Matroska) rather than a generic property of raw video streams - the extradata string is an ffmpeg invention. Since this option is meant to pacify old legacy apps which wouldn't deal with multiple streams, per-stream assignment is a bonus but not necessary. I modified Matroska muxer for completeness rather than expecting anyone to actually use it. Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v5] avdevice/xcbgrab: check return values of xcb query functions
On Sun, Jul 12, 2020 at 10:54:45 -0400, Andriy Gelman wrote:
> On Fri, 10. Jul 21:13, Moritz Barsnick wrote:
> > Since xcbgrab is getting some attention recently...
> >
> > Fixes a segfault, as reported in #7312.
> >
> > To reproduce:
> > Terminal 1:
> > $ Xvfb :1 -nolisten tcp -screen 0 800x600x24
> > Terminal 2:
> > $ ffmpeg -f x11grab -i :1 -f null -
> > or rather
> > $ gdb -ex r --args ffmpeg_g -f x11grab -i :1 -f null -
> > Then terminate Xvfb while ffmpeg is running.
>
> The rest of the av_log calls use AVFormatContext*.
> You may want to update to be consistent.
Good point. I didn't mind the "xcbgrab indev" vs. "x11grab", but it's
indeed inconsistent. Updated patch attached.
Moritz
From 269b43209394c0eceb83f5ae384792c32305333a Mon Sep 17 00:00:00 2001
From: Moritz Barsnick
Date: Tue, 14 Jul 2020 14:07:33 +0200
Subject: [PATCH] avdevice/xcbgrab: check return values of xcb query functions
Fixes #7312, segmentation fault on close of X11 server
xcb_query_pointer_reply() and xcb_get_geometry_reply() can return NULL
if e.g. the X server closes or the connection is lost. This needs to
be checked in order to cleanly exit, because the returned pointers are
dereferenced later.
Furthermore, their return values need to be free()d, also in error
code paths.
Signed-off-by: Moritz Barsnick
---
libavdevice/xcbgrab.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/libavdevice/xcbgrab.c b/libavdevice/xcbgrab.c
index 6f6b2dbf15..8bc320d055 100644
--- a/libavdevice/xcbgrab.c
+++ b/libavdevice/xcbgrab.c
@@ -346,8 +346,10 @@ static void xcbgrab_draw_mouse(AVFormatContext *s,
AVPacket *pkt,
return;
cursor = xcb_xfixes_get_cursor_image_cursor_image(ci);
-if (!cursor)
+if (!cursor) {
+free(ci);
return;
+}
cx = ci->x - ci->xhot;
cy = ci->y - ci->yhot;
@@ -425,7 +427,16 @@ static int xcbgrab_read_packet(AVFormatContext *s,
AVPacket *pkt)
pc = xcb_query_pointer(c->conn, c->screen->root);
gc = xcb_get_geometry(c->conn, c->screen->root);
p = xcb_query_pointer_reply(c->conn, pc, NULL);
+if (!p) {
+av_log(s, AV_LOG_ERROR, "Failed to query xcb pointer\n");
+return AVERROR_EXTERNAL;
+}
geo = xcb_get_geometry_reply(c->conn, gc, NULL);
+if (!geo) {
+av_log(s, AV_LOG_ERROR, "Failed to get xcb geometry\n");
+free(p);
+return AVERROR_EXTERNAL;
+}
}
if (c->follow_mouse && p->same_screen)
--
2.26.2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/riffenc: indicate storage of flipped RGB bitmaps
Gyan Doshi: > Some legacy applications such as AVI2MVE expect raw RGB bitmaps > to be stored bottom-up, whereas our RIFF BITMAPINFOHEADER assumes > they are always stored top-down and thus write a negative value > for height. This can prevent reading of these files. > > Option flipped_raw_rgb added to AVI and Matroska muxers > which will write positive value for height when enabled. > > Note that the user has to flip the bitmaps beforehand using other > means such as the vflip filter. > --- 1. The avi demuxer adds "BottomUp" to the extradata in case it demuxes bottom up data. The Matroska demuxer currently doesn't and so rawvideo stored in Matroska in vfw mode will always be treated as top-down. This needs to be fixed, too. 2. But why don't we add an option to the raw encoder so that it adds (or maybe switches?) "BottomUp" to the extradata to indicate that the output is bottom-up? That seems like a better place for me. (E.g. it would naturally allow to set the bottom-up/top-down property on a per-stream basis.) - Andreas ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v9 3/3] avdevice/decklink_dec: export timecode with s12m side data
On Mon, Jul 13, 2020 at 12:46:17PM +0800, lance.lmw...@gmail.com wrote:
> From: Limin Wang
>
> Reviewed-by: Marton Balint
> Signed-off-by: Limin Wang
> ---
> libavdevice/decklink_dec.cpp | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/libavdevice/decklink_dec.cpp b/libavdevice/decklink_dec.cpp
> index a499972..dde68ff 100644
> --- a/libavdevice/decklink_dec.cpp
> +++ b/libavdevice/decklink_dec.cpp
> @@ -42,6 +42,7 @@ extern "C" {
> #include "libavutil/imgutils.h"
> #include "libavutil/intreadwrite.h"
> #include "libavutil/time.h"
> +#include "libavutil/timecode.h"
> #include "libavutil/mathematics.h"
> #include "libavutil/reverse.h"
> #include "avdevice.h"
> @@ -882,6 +883,19 @@ HRESULT decklink_input_callback::VideoInputFrameArrived(
> AVDictionary* metadata_dict = NULL;
> int metadata_len;
> uint8_t* packed_metadata;
> +AVTimecode tcr;
> +
> +if (av_timecode_init_from_string(,
> ctx->video_st->r_frame_rate, tc, ctx) >= 0) {
> +uint32_t tc_data =
> av_timecode_get_smpte_from_framenum(, 0);
> +int size = sizeof(uint32_t) * 4;
> +uint32_t *sd = (uint32_t
> *)av_packet_new_side_data(, AV_PKT_DATA_S12M_TIMECODE, size);
> +
> +if (sd) {
> +*sd = 1; // one TC
> +*(sd + 1) = tc_data; // TC
> +}
> +}
> +
> if (av_dict_set(_dict, "timecode", tc,
> AV_DICT_DONT_STRDUP_VAL) >= 0) {
> packed_metadata =
> av_packet_pack_dictionary(metadata_dict, _len);
> av_dict_free(_dict);
> --
> 1.8.3.1
>
will apply the patchset tomorrow if no more comments. I have other change to
support > 30FPS
and plan to submit them in separate patch later.
--
Thanks,
Limin Wang
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH v2] avformat/crypto.c: remove unnecessary code
tis 2020-07-14 klockan 14:23 +0800 skrev Steven Liu:
> Because the newpos variable is set value before use it.
> The newpos variable declared at the head partition of crypto_seek.
> Make the code clean.
>
> Signed-off-by: Steven Liu
> ---
> libavformat/crypto.c | 12
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/libavformat/crypto.c b/libavformat/crypto.c
> index 31f9ac0ab9..1d4514e0f2 100644
> --- a/libavformat/crypto.c
> +++ b/libavformat/crypto.c
> @@ -252,21 +252,17 @@ static int64_t crypto_seek(URLContext *h, int64_t pos,
> int whence)
> case SEEK_CUR:
> pos = pos + c->position;
> break;
> -case SEEK_END: {
> -int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
> +case SEEK_END:
> +newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
> if (newpos < 0) {
> av_log(h, AV_LOG_ERROR,
> "Crypto: seek_end - can't get file size (pos=%lld)\r\n",
> (long long int)pos);
> return newpos;
> }
> pos = newpos - pos;
> -}
> -break;
> -case AVSEEK_SIZE: {
> -int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
> -return newpos;
> -}
> break;
> +case AVSEEK_SIZE:
> +return ffurl_seek( c->hd, pos, AVSEEK_SIZE );
Looks OK enough.
/Tomas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avformat/riffenc: indicate storage of flipped RGB bitmaps
On 14-07-2020 01:54 pm, Paul B Mahol wrote:
Why is option called rgb?
It have nothing to do with rgb.
Flipped can be any encoded file.
No, coded or uncoded streams with a codec tag or raw YUV bitmaps are
always indicated as positive height. Only raw RGB bitmaps can be
signaled as flipped.
See doc for |biHeight in
https://docs.microsoft.com/en-us/windows/win32/api/wingdi/ns-wingdi-bitmapinfoheader#members
or the end of this page
https://docs.microsoft.com/en-us/windows/win32/directshow/top-down-vs--bottom-up-dibs
Regards,
Gyan
|
On 7/14/20, Gyan Doshi wrote:
Plan to push in 24h.
On 08-07-2020 06:21 pm, Gyan Doshi wrote:
Some legacy applications such as AVI2MVE expect raw RGB bitmaps
to be stored bottom-up, whereas our RIFF BITMAPINFOHEADER assumes
they are always stored top-down and thus write a negative value
for height. This can prevent reading of these files.
Option flipped_raw_rgb added to AVI and Matroska muxers
which will write positive value for height when enabled.
Note that the user has to flip the bitmaps beforehand using other
means such as the vflip filter.
---
doc/muxers.texi | 13 +
libavformat/asfenc.c | 2 +-
libavformat/avienc.c | 4 +++-
libavformat/matroskaenc.c | 5 -
libavformat/riff.h| 2 +-
libavformat/riffenc.c | 7 ---
libavformat/wtvenc.c | 2 +-
7 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index b1389a3227..2f26494bfa 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -89,6 +89,12 @@ specific scenarios, e.g. when merging multiple audio
streams into one for
compatibility with software that only supports a single audio stream in
AVI
(see @ref{amerge,,the "amerge" section in the ffmpeg-filters
manual,ffmpeg-filters}).
+@item flipped_raw_rgb
+If set to true, store positive height for raw RGB bitmaps, which
indicates
+bitmap is stored bottom-up. Note that this option does not flip the
bitmap
+which has to be done manually beforehand, e.g. by using the vflip filter.
+Default is @var{false} and indicates bitmap is stored top down.
+
@end table
@anchor{chromaprint}
@@ -1409,6 +1415,13 @@ disposition default exists, no subtitle track will
be marked as default.
In this mode the FlagDefault is set if and only if the
AV_DISPOSITION_DEFAULT
flag is set in the disposition of the corresponding stream.
@end table
+
+@item flipped_raw_rgb
+If set to true, store positive height for raw RGB bitmaps, which
indicates
+bitmap is stored bottom-up. Note that this option does not flip the
bitmap
+which has to be done manually beforehand, e.g. by using the vflip filter.
+Default is @var{false} and indicates bitmap is stored top down.
+
@end table
@anchor{md5}
diff --git a/libavformat/asfenc.c b/libavformat/asfenc.c
index 73afb13200..8b24264c94 100644
--- a/libavformat/asfenc.c
+++ b/libavformat/asfenc.c
@@ -682,7 +682,7 @@ static int asf_write_header1(AVFormatContext *s,
int64_t file_size,
avio_wl16(pb, 40 + par->extradata_size); /* size */
/* BITMAPINFOHEADER header */
-ff_put_bmp_header(pb, par, 1, 0);
+ff_put_bmp_header(pb, par, 1, 0, 0);
}
end_header(pb, hpos);
}
diff --git a/libavformat/avienc.c b/libavformat/avienc.c
index 297d5b8964..1b2cb529b9 100644
--- a/libavformat/avienc.c
+++ b/libavformat/avienc.c
@@ -72,6 +72,7 @@ typedef struct AVIContext {
int reserve_index_space;
int master_index_max_size;
int write_channel_mask;
+int flipped_raw_rgb;
} AVIContext;
typedef struct AVIStream {
@@ -449,7 +450,7 @@ static int avi_write_header(AVFormatContext *s)
&& par->bits_per_coded_sample == 15)
par->bits_per_coded_sample = 16;
avist->pal_offset = avio_tell(pb) + 40;
-ff_put_bmp_header(pb, par, 0, 0);
+ff_put_bmp_header(pb, par, 0, 0, avi->flipped_raw_rgb);
pix_fmt = avpriv_find_pix_fmt(avpriv_pix_fmt_bps_avi,
par->bits_per_coded_sample);
if ( !par->codec_tag
@@ -993,6 +994,7 @@ static void avi_deinit(AVFormatContext *s)
static const AVOption options[] = {
{ "reserve_index_space", "reserve space (in bytes) at the beginning
of the file for each stream index", OFFSET(reserve_index_space),
AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, ENC },
{ "write_channel_mask", "write channel mask into wave format
header", OFFSET(write_channel_mask), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1,
ENC },
+{ "flipped_raw_rgb", "Raw RGB bitmaps are stored bottom-up",
OFFSET(flipped_raw_rgb), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, ENC },
{ NULL },
};
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 105ed5197e..233c472b8f 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -154,6 +154,7 @@ typedef struct MatroskaMuxContext {
Re: [FFmpeg-devel] [PATCH] avformat/riffenc: indicate storage of flipped RGB bitmaps
Why is option called rgb?
It have nothing to do with rgb.
Flipped can be any encoded file.
On 7/14/20, Gyan Doshi wrote:
> Plan to push in 24h.
>
> On 08-07-2020 06:21 pm, Gyan Doshi wrote:
>> Some legacy applications such as AVI2MVE expect raw RGB bitmaps
>> to be stored bottom-up, whereas our RIFF BITMAPINFOHEADER assumes
>> they are always stored top-down and thus write a negative value
>> for height. This can prevent reading of these files.
>>
>> Option flipped_raw_rgb added to AVI and Matroska muxers
>> which will write positive value for height when enabled.
>>
>> Note that the user has to flip the bitmaps beforehand using other
>> means such as the vflip filter.
>> ---
>> doc/muxers.texi | 13 +
>> libavformat/asfenc.c | 2 +-
>> libavformat/avienc.c | 4 +++-
>> libavformat/matroskaenc.c | 5 -
>> libavformat/riff.h| 2 +-
>> libavformat/riffenc.c | 7 ---
>> libavformat/wtvenc.c | 2 +-
>> 7 files changed, 27 insertions(+), 8 deletions(-)
>>
>> diff --git a/doc/muxers.texi b/doc/muxers.texi
>> index b1389a3227..2f26494bfa 100644
>> --- a/doc/muxers.texi
>> +++ b/doc/muxers.texi
>> @@ -89,6 +89,12 @@ specific scenarios, e.g. when merging multiple audio
>> streams into one for
>> compatibility with software that only supports a single audio stream in
>> AVI
>> (see @ref{amerge,,the "amerge" section in the ffmpeg-filters
>> manual,ffmpeg-filters}).
>>
>> +@item flipped_raw_rgb
>> +If set to true, store positive height for raw RGB bitmaps, which
>> indicates
>> +bitmap is stored bottom-up. Note that this option does not flip the
>> bitmap
>> +which has to be done manually beforehand, e.g. by using the vflip filter.
>> +Default is @var{false} and indicates bitmap is stored top down.
>> +
>> @end table
>>
>> @anchor{chromaprint}
>> @@ -1409,6 +1415,13 @@ disposition default exists, no subtitle track will
>> be marked as default.
>> In this mode the FlagDefault is set if and only if the
>> AV_DISPOSITION_DEFAULT
>> flag is set in the disposition of the corresponding stream.
>> @end table
>> +
>> +@item flipped_raw_rgb
>> +If set to true, store positive height for raw RGB bitmaps, which
>> indicates
>> +bitmap is stored bottom-up. Note that this option does not flip the
>> bitmap
>> +which has to be done manually beforehand, e.g. by using the vflip filter.
>> +Default is @var{false} and indicates bitmap is stored top down.
>> +
>> @end table
>>
>> @anchor{md5}
>> diff --git a/libavformat/asfenc.c b/libavformat/asfenc.c
>> index 73afb13200..8b24264c94 100644
>> --- a/libavformat/asfenc.c
>> +++ b/libavformat/asfenc.c
>> @@ -682,7 +682,7 @@ static int asf_write_header1(AVFormatContext *s,
>> int64_t file_size,
>> avio_wl16(pb, 40 + par->extradata_size); /* size */
>>
>> /* BITMAPINFOHEADER header */
>> -ff_put_bmp_header(pb, par, 1, 0);
>> +ff_put_bmp_header(pb, par, 1, 0, 0);
>> }
>> end_header(pb, hpos);
>> }
>> diff --git a/libavformat/avienc.c b/libavformat/avienc.c
>> index 297d5b8964..1b2cb529b9 100644
>> --- a/libavformat/avienc.c
>> +++ b/libavformat/avienc.c
>> @@ -72,6 +72,7 @@ typedef struct AVIContext {
>> int reserve_index_space;
>> int master_index_max_size;
>> int write_channel_mask;
>> +int flipped_raw_rgb;
>> } AVIContext;
>>
>> typedef struct AVIStream {
>> @@ -449,7 +450,7 @@ static int avi_write_header(AVFormatContext *s)
>> && par->bits_per_coded_sample == 15)
>> par->bits_per_coded_sample = 16;
>> avist->pal_offset = avio_tell(pb) + 40;
>> -ff_put_bmp_header(pb, par, 0, 0);
>> +ff_put_bmp_header(pb, par, 0, 0, avi->flipped_raw_rgb);
>> pix_fmt = avpriv_find_pix_fmt(avpriv_pix_fmt_bps_avi,
>>
>> par->bits_per_coded_sample);
>> if ( !par->codec_tag
>> @@ -993,6 +994,7 @@ static void avi_deinit(AVFormatContext *s)
>> static const AVOption options[] = {
>> { "reserve_index_space", "reserve space (in bytes) at the beginning
>> of the file for each stream index", OFFSET(reserve_index_space),
>> AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, ENC },
>> { "write_channel_mask", "write channel mask into wave format
>> header", OFFSET(write_channel_mask), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1,
>> ENC },
>> +{ "flipped_raw_rgb", "Raw RGB bitmaps are stored bottom-up",
>> OFFSET(flipped_raw_rgb), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, ENC },
>> { NULL },
>> };
>>
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 105ed5197e..233c472b8f 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -154,6 +154,7 @@ typedef struct MatroskaMuxContext {
>> int is_dash;
>> int dash_track_number;
>> int allow_raw_vfw;
>> +int
Re: [FFmpeg-devel] [PATCH] avformat/riffenc: indicate storage of flipped RGB bitmaps
Plan to push in 24h.
On 08-07-2020 06:21 pm, Gyan Doshi wrote:
Some legacy applications such as AVI2MVE expect raw RGB bitmaps
to be stored bottom-up, whereas our RIFF BITMAPINFOHEADER assumes
they are always stored top-down and thus write a negative value
for height. This can prevent reading of these files.
Option flipped_raw_rgb added to AVI and Matroska muxers
which will write positive value for height when enabled.
Note that the user has to flip the bitmaps beforehand using other
means such as the vflip filter.
---
doc/muxers.texi | 13 +
libavformat/asfenc.c | 2 +-
libavformat/avienc.c | 4 +++-
libavformat/matroskaenc.c | 5 -
libavformat/riff.h| 2 +-
libavformat/riffenc.c | 7 ---
libavformat/wtvenc.c | 2 +-
7 files changed, 27 insertions(+), 8 deletions(-)
diff --git a/doc/muxers.texi b/doc/muxers.texi
index b1389a3227..2f26494bfa 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -89,6 +89,12 @@ specific scenarios, e.g. when merging multiple audio streams
into one for
compatibility with software that only supports a single audio stream in AVI
(see @ref{amerge,,the "amerge" section in the ffmpeg-filters
manual,ffmpeg-filters}).
+@item flipped_raw_rgb
+If set to true, store positive height for raw RGB bitmaps, which indicates
+bitmap is stored bottom-up. Note that this option does not flip the bitmap
+which has to be done manually beforehand, e.g. by using the vflip filter.
+Default is @var{false} and indicates bitmap is stored top down.
+
@end table
@anchor{chromaprint}
@@ -1409,6 +1415,13 @@ disposition default exists, no subtitle track will be
marked as default.
In this mode the FlagDefault is set if and only if the AV_DISPOSITION_DEFAULT
flag is set in the disposition of the corresponding stream.
@end table
+
+@item flipped_raw_rgb
+If set to true, store positive height for raw RGB bitmaps, which indicates
+bitmap is stored bottom-up. Note that this option does not flip the bitmap
+which has to be done manually beforehand, e.g. by using the vflip filter.
+Default is @var{false} and indicates bitmap is stored top down.
+
@end table
@anchor{md5}
diff --git a/libavformat/asfenc.c b/libavformat/asfenc.c
index 73afb13200..8b24264c94 100644
--- a/libavformat/asfenc.c
+++ b/libavformat/asfenc.c
@@ -682,7 +682,7 @@ static int asf_write_header1(AVFormatContext *s, int64_t
file_size,
avio_wl16(pb, 40 + par->extradata_size); /* size */
/* BITMAPINFOHEADER header */
-ff_put_bmp_header(pb, par, 1, 0);
+ff_put_bmp_header(pb, par, 1, 0, 0);
}
end_header(pb, hpos);
}
diff --git a/libavformat/avienc.c b/libavformat/avienc.c
index 297d5b8964..1b2cb529b9 100644
--- a/libavformat/avienc.c
+++ b/libavformat/avienc.c
@@ -72,6 +72,7 @@ typedef struct AVIContext {
int reserve_index_space;
int master_index_max_size;
int write_channel_mask;
+int flipped_raw_rgb;
} AVIContext;
typedef struct AVIStream {
@@ -449,7 +450,7 @@ static int avi_write_header(AVFormatContext *s)
&& par->bits_per_coded_sample == 15)
par->bits_per_coded_sample = 16;
avist->pal_offset = avio_tell(pb) + 40;
-ff_put_bmp_header(pb, par, 0, 0);
+ff_put_bmp_header(pb, par, 0, 0, avi->flipped_raw_rgb);
pix_fmt = avpriv_find_pix_fmt(avpriv_pix_fmt_bps_avi,
par->bits_per_coded_sample);
if ( !par->codec_tag
@@ -993,6 +994,7 @@ static void avi_deinit(AVFormatContext *s)
static const AVOption options[] = {
{ "reserve_index_space", "reserve space (in bytes) at the beginning of the
file for each stream index", OFFSET(reserve_index_space), AV_OPT_TYPE_INT, { .i64 = 0 }, 0,
INT_MAX, ENC },
{ "write_channel_mask", "write channel mask into wave format header",
OFFSET(write_channel_mask), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, ENC },
+{ "flipped_raw_rgb", "Raw RGB bitmaps are stored bottom-up",
OFFSET(flipped_raw_rgb), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, ENC },
{ NULL },
};
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index 105ed5197e..233c472b8f 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -154,6 +154,7 @@ typedef struct MatroskaMuxContext {
int is_dash;
int dash_track_number;
int allow_raw_vfw;
+int flipped_raw_rgb;
int default_mode;
uint32_tsegment_uid[4];
@@ -764,6 +765,7 @@ static int mkv_write_codecprivate(AVFormatContext *s,
AVIOContext *pb,
int native_id, int qt_id)
{
AVIOContext *dyn_cp;
+MatroskaMuxContext *mkv = s->priv_data;
uint8_t *codecpriv;
int ret, codecpriv_size;
@@ -802,7
Re: [FFmpeg-devel] [PATCH 2/2] swscale/tests/swscale: Initialize res to a non random error code
On Sun, 12 Jul 2020, Michael Niedermayer wrote:
Regression since: 3adffab073bc59af39035168ac72bc9ffde3
-1 is consistent what other error paths return
Signed-off-by: Michael Niedermayer
---
libswscale/tests/swscale.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libswscale/tests/swscale.c b/libswscale/tests/swscale.c
index 58870fdb78..845ced61bb 100644
--- a/libswscale/tests/swscale.c
+++ b/libswscale/tests/swscale.c
@@ -423,8 +423,10 @@ bad_option:
for (x = 0; x < W * 4; x++)
rgb_data[ x + y * 4 * W] = av_lfg_get();
res = sws_scale(sws, rgb_src, rgb_stride, 0, H / 12, (uint8_t * const *)
src, stride);
-if (res < 0 || res != H)
+if (res < 0 || res != H) {
+res = -1;
goto error;
+}
sws_freeContext(sws);
av_free(rgb_data);
LGTM
Do note that for the return value of a process is limited to the range
0-255 normally, and if returning a negative value, it gets mapped to the
upper range (128-255) which normally is used for when a process is killed
by a signal. But this issue seems to already be present within this test
app, and this change makes it internally consistent at least.
// Martin
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH 1/2] swscale/tests/swscale: Fix incorrect return code check
On Sun, 12 Jul 2020, Michael Niedermayer wrote: Regression since: 3adffab073bc59af39035168ac72bc9ffde3 Signed-off-by: Michael Niedermayer --- libswscale/tests/swscale.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libswscale/tests/swscale.c b/libswscale/tests/swscale.c index 693f439bf5..58870fdb78 100644 --- a/libswscale/tests/swscale.c +++ b/libswscale/tests/swscale.c @@ -423,7 +423,7 @@ bad_option: for (x = 0; x < W * 4; x++) rgb_data[ x + y * 4 * W] = av_lfg_get(); res = sws_scale(sws, rgb_src, rgb_stride, 0, H / 12, (uint8_t * const *) src, stride); -if (res < 0 || res != (H / 12)) +if (res < 0 || res != H) goto error; LGTM, this seems to fix the test on both aarch64 and x86. // Martin ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
[FFmpeg-devel] [PATCH v2] avformat/crypto.c: remove unnecessary code
Because the newpos variable is set value before use it.
The newpos variable declared at the head partition of crypto_seek.
Make the code clean.
Signed-off-by: Steven Liu
---
libavformat/crypto.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/libavformat/crypto.c b/libavformat/crypto.c
index 31f9ac0ab9..1d4514e0f2 100644
--- a/libavformat/crypto.c
+++ b/libavformat/crypto.c
@@ -252,21 +252,17 @@ static int64_t crypto_seek(URLContext *h, int64_t pos,
int whence)
case SEEK_CUR:
pos = pos + c->position;
break;
-case SEEK_END: {
-int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
+case SEEK_END:
+newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
if (newpos < 0) {
av_log(h, AV_LOG_ERROR,
"Crypto: seek_end - can't get file size (pos=%lld)\r\n", (long
long int)pos);
return newpos;
}
pos = newpos - pos;
-}
-break;
-case AVSEEK_SIZE: {
-int64_t newpos = ffurl_seek( c->hd, pos, AVSEEK_SIZE );
-return newpos;
-}
break;
+case AVSEEK_SIZE:
+return ffurl_seek( c->hd, pos, AVSEEK_SIZE );
default:
av_log(h, AV_LOG_ERROR,
"Crypto: no support for seek where 'whence' is %d\r\n", whence);
--
2.25.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-requ...@ffmpeg.org with subject "unsubscribe".
