Re: [FFmpeg-devel] [PATCH 2/2] avfilter/vf_lut3d: prelut support for 3d cinespace luts
On Thu, May 21, 2020 at 8:14 AM Paul B Mahol wrote: > Probably ok, except code style. > Please keep code style consistent across files. > Thanks a lot for the feedback. I see a few inconsistently named variables, maybe I used too many spaces in places. Could you tell me where is it that you have the most issue with? Did I name functions incorrectly or put them in bad locations? > On 5/19/20, mindm...@gmail.com wrote: > > From: Mark Reid > > > > --- > > libavfilter/vf_lut3d.c | 367 +++-- > > 1 file changed, 312 insertions(+), 55 deletions(-) > > > ___ 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/3] avformat/matroskaenc: Don't use stream side-data size
James Almer: > On 5/22/2020 12:31 AM, James Almer wrote: >> On 5/21/2020 10:24 PM, Andreas Rheinhardt wrote: >>> av_stream_get_side_data() tells the caller whether a stream has side >>> data of a specific type; if present it can also tell the caller the size >>> of the side data via an optional argument. The Matroska muxer always >>> used this optional argument, although it doesn't really need the size, >>> as the relevant side-data are not buffers, but structures. So change >>> this. >>> >>> Furthermore, relying on the size also made the code susceptible to >>> a quirk of av_stream_get_side_data(): It only sets the size argument if >>> it found side data of the desired type. >> >> Sounds like something that should be fixed instead. >> av_packet_get_side_data() sets the size argument to 0 if it doesn't find >> the requested side data type. This function should do the same. > > Right, you did as much in patch 3/3. > > This patch is worth applying either way seeing it removes things like > the API violating usage of sizeof(AVMasteringDisplayMetadata), so LGTM. Ok, applied the set. Thanks. - 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 1/3] avformat/matroskaenc: Don't use stream side-data size
On 5/22/2020 12:31 AM, James Almer wrote: > On 5/21/2020 10:24 PM, Andreas Rheinhardt wrote: >> av_stream_get_side_data() tells the caller whether a stream has side >> data of a specific type; if present it can also tell the caller the size >> of the side data via an optional argument. The Matroska muxer always >> used this optional argument, although it doesn't really need the size, >> as the relevant side-data are not buffers, but structures. So change >> this. >> >> Furthermore, relying on the size also made the code susceptible to >> a quirk of av_stream_get_side_data(): It only sets the size argument if >> it found side data of the desired type. > > Sounds like something that should be fixed instead. > av_packet_get_side_data() sets the size argument to 0 if it doesn't find > the requested side data type. This function should do the same. Right, you did as much in patch 3/3. This patch is worth applying either way seeing it removes things like the API violating usage of sizeof(AVMasteringDisplayMetadata), so LGTM. ___ 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/3] avformat/matroskaenc: Remove pointless casts
On 5/21/2020 10:24 PM, Andreas Rheinhardt wrote:
> by using a const void * pointer as an intermediate.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> Why is the side-data API (both the packet as well as the stream one)
> actually based around uint8_t * and not pointers to void despite
> side-data being mostly structures and not just buffers?
uint8_t* is ubiquitous in the codebase for pointers to allocated data.
And if you look at the early side data types (palette, new extradata),
it's not surprising they were defined this way (Assuming these functions
are as old as the very first side data types).
>
> libavformat/matroskaenc.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index f5968c17b4..9ad590cb93 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -836,7 +836,7 @@ static void mkv_write_video_color(AVIOContext *pb, const
> AVStream *st,
> uint8_t colour[(2 + 1 + 8) * 18 + (2 + 1) + 1];
> AVIOContext buf, *dyn_cp = &buf;
> int colorinfo_size;
> -const uint8_t *side_data;
> +const void *side_data;
>
> ffio_init_context(dyn_cp, colour, sizeof(colour), 1, NULL, NULL, NULL,
> NULL);
>
> @@ -869,8 +869,7 @@ static void mkv_write_video_color(AVIOContext *pb, const
> AVStream *st,
> side_data = av_stream_get_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
> NULL);
> if (side_data) {
> -const AVContentLightMetadata *metadata =
> -(const AVContentLightMetadata*)side_data;
> +const AVContentLightMetadata *metadata = side_data;
> put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOCOLORMAXCLL,
> metadata->MaxCLL);
> put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOCOLORMAXFALL,
> metadata->MaxFALL);
> }
> @@ -880,8 +879,7 @@ static void mkv_write_video_color(AVIOContext *pb, const
> AVStream *st,
> if (side_data) {
> ebml_master meta_element = start_ebml_master(
> dyn_cp, MATROSKA_ID_VIDEOCOLORMASTERINGMETA, 10 * (2 + 1 + 8));
> -const AVMasteringDisplayMetadata *metadata =
> -(const AVMasteringDisplayMetadata*)side_data;
> +const AVMasteringDisplayMetadata *metadata = side_data;
> if (metadata->has_primaries) {
> put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOCOLOR_RX,
> av_q2d(metadata->display_primaries[0][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 1/3] avformat/matroskaenc: Don't use stream side-data size
James Almer: > On 5/21/2020 10:24 PM, Andreas Rheinhardt wrote: >> av_stream_get_side_data() tells the caller whether a stream has side >> data of a specific type; if present it can also tell the caller the size >> of the side data via an optional argument. The Matroska muxer always >> used this optional argument, although it doesn't really need the size, >> as the relevant side-data are not buffers, but structures. So change >> this. >> >> Furthermore, relying on the size also made the code susceptible to >> a quirk of av_stream_get_side_data(): It only sets the size argument if >> it found side data of the desired type. > > Sounds like something that should be fixed instead. > av_packet_get_side_data() sets the size argument to 0 if it doesn't find > the requested side data type. This function should do the same. > The third patch [1] in this patchset does exactly this. - Andreas [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2020-May/263088.html ___ 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/3] avformat/matroskaenc: Don't use stream side-data size
On 5/21/2020 10:24 PM, Andreas Rheinhardt wrote:
> av_stream_get_side_data() tells the caller whether a stream has side
> data of a specific type; if present it can also tell the caller the size
> of the side data via an optional argument. The Matroska muxer always
> used this optional argument, although it doesn't really need the size,
> as the relevant side-data are not buffers, but structures. So change
> this.
>
> Furthermore, relying on the size also made the code susceptible to
> a quirk of av_stream_get_side_data(): It only sets the size argument if
> it found side data of the desired type.
Sounds like something that should be fixed instead.
av_packet_get_side_data() sets the size argument to 0 if it doesn't find
the requested side data type. This function should do the same.
> mkv_write_video_color() checks
> for side-data twice with the same variable for the size without resetting
> the size in between; if the second type of side-data isn't present, the
> size will still be what it was after the first call. This was not
> dangerous in practice, as the check for the existence of the second
> side-data compared the size with the expected size, so it would only be
> problematic if lots of elements were to be added to AVContentLightMetadata.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/matroskaenc.c | 19 ---
> 1 file changed, 8 insertions(+), 11 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index fccfee539a..f5968c17b4 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -835,7 +835,6 @@ static void mkv_write_video_color(AVIOContext *pb, const
> AVStream *st,
> * plus another byte to stay clear of the end. */
> uint8_t colour[(2 + 1 + 8) * 18 + (2 + 1) + 1];
> AVIOContext buf, *dyn_cp = &buf;
> -int side_data_size = 0;
> int colorinfo_size;
> const uint8_t *side_data;
>
> @@ -868,8 +867,8 @@ static void mkv_write_video_color(AVIOContext *pb, const
> AVStream *st,
> }
>
> side_data = av_stream_get_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
> -&side_data_size);
> -if (side_data_size) {
> +NULL);
> +if (side_data) {
> const AVContentLightMetadata *metadata =
> (const AVContentLightMetadata*)side_data;
> put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOCOLORMAXCLL,
> metadata->MaxCLL);
> @@ -877,8 +876,8 @@ static void mkv_write_video_color(AVIOContext *pb, const
> AVStream *st,
> }
>
> side_data = av_stream_get_side_data(st,
> AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
> -&side_data_size);
> -if (side_data_size == sizeof(AVMasteringDisplayMetadata)) {
> +NULL);
> +if (side_data) {
> ebml_master meta_element = start_ebml_master(
> dyn_cp, MATROSKA_ID_VIDEOCOLORMASTERINGMETA, 10 * (2 + 1 + 8));
> const AVMasteringDisplayMetadata *metadata =
> @@ -919,14 +918,13 @@ static void mkv_write_video_projection(AVFormatContext
> *s, AVIOContext *pb,
> const AVStream *st)
> {
> ebml_master projection;
> -int side_data_size = 0;
> uint8_t private[20];
>
> const AVSphericalMapping *spherical =
> (const AVSphericalMapping *)av_stream_get_side_data(st,
> AV_PKT_DATA_SPHERICAL,
> -&side_data_size);
> +NULL);
>
> -if (!side_data_size)
> +if (!spherical)
> return;
>
> if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR &&
> @@ -1028,7 +1026,6 @@ static int mkv_write_stereo_mode(AVFormatContext *s,
> AVIOContext *pb,
> const AVDictionaryEntry *tag;
> MatroskaVideoStereoModeType format = MATROSKA_VIDEO_STEREOMODE_TYPE_NB;
> const AVStereo3D *stereo;
> -int side_data_size = 0;
>
> *h_width = 1;
> *h_height = 1;
> @@ -1052,8 +1049,8 @@ static int mkv_write_stereo_mode(AVFormatContext *s,
> AVIOContext *pb,
> }
>
> stereo = (const AVStereo3D*)av_stream_get_side_data(st,
> AV_PKT_DATA_STEREO3D,
> -&side_data_size);
> -if (side_data_size >= sizeof(AVStereo3D)) {
> +NULL);
> +if (stereo) {
> switch (stereo->type) {
> case AV_STEREO3D_2D:
> format = MATROSKA_VIDEO_STEREOMODE_TYPE_MONO;
>
___
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/5] avformat/webmdashenc: Avoid allocation for parsing a number
Andreas Rheinhardt:
> In order to parse a number from a string, the WebM DASH manifest muxer
> would duplicate (via heap-allocation) the part of the string that
> contains the number, then read the number via atoi() and then free the
> duplicate again. This has been replaced by simply using strtoll() (which
> in contrast to atoi() has defined behaviour when the number is not
> representable).
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/webmdashenc.c | 14 +-
> 1 file changed, 1 insertion(+), 13 deletions(-)
>
> diff --git a/libavformat/webmdashenc.c b/libavformat/webmdashenc.c
> index 465485c90c..05015a08c1 100644
> --- a/libavformat/webmdashenc.c
> +++ b/libavformat/webmdashenc.c
> @@ -425,18 +425,6 @@ static int write_adaptation_set(AVFormatContext *s, int
> as_index)
> return 0;
> }
>
> -static int to_integer(char *p, int len)
> -{
> -int ret;
> -char *q = av_malloc(len);
> -if (!q)
> -return AVERROR(ENOMEM);
> -av_strlcpy(q, p, len);
> -ret = atoi(q);
> -av_free(q);
> -return ret;
> -}
> -
> static int parse_adaptation_sets(AVFormatContext *s)
> {
> WebMDashMuxContext *w = s->priv_data;
> @@ -483,7 +471,7 @@ static int parse_adaptation_sets(AVFormatContext *s)
> return ret;
> q = p;
> while (*q != '\0' && *q != ',' && *q != ' ') q++;
> -as->streams[as->nb_streams - 1] = to_integer(p, q - p + 1);
> +as->streams[as->nb_streams - 1] = strtoll(p, NULL, 10);
> if (as->streams[as->nb_streams - 1] < 0 ||
> as->streams[as->nb_streams - 1] >= s->nb_streams) {
> av_log(s, AV_LOG_ERROR, "Invalid value for 'streams' in
> adapation_sets.\n");
>
Will apply this patchset tomorrow unless there are objections.
- 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 1/6] avformat/segment: Access AVStream more directly
Andreas Rheinhardt:
> by storing s->streams[i] in a pointer instead of constantly using
> s->streams[i]->...
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/segment.c | 17 -
> 1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/libavformat/segment.c b/libavformat/segment.c
> index 32c09827eb..7e8bd65976 100644
> --- a/libavformat/segment.c
> +++ b/libavformat/segment.c
> @@ -162,12 +162,11 @@ static int segment_mux_init(AVFormatContext *s)
> oc->flags = s->flags;
>
> for (i = 0; i < s->nb_streams; i++) {
> -AVStream *st;
> -AVCodecParameters *ipar, *opar;
> +AVStream *st, *ist = s->streams[i];
> +AVCodecParameters *ipar = ist->codecpar, *opar;
>
> if (!(st = avformat_new_stream(oc, NULL)))
> return AVERROR(ENOMEM);
> -ipar = s->streams[i]->codecpar;
> opar = st->codecpar;
> avcodec_parameters_copy(opar, ipar);
> if (!oc->oformat->codec_tag ||
> @@ -177,16 +176,16 @@ static int segment_mux_init(AVFormatContext *s)
> } else {
> opar->codec_tag = 0;
> }
> -st->sample_aspect_ratio = s->streams[i]->sample_aspect_ratio;
> -st->time_base = s->streams[i]->time_base;
> -st->avg_frame_rate = s->streams[i]->avg_frame_rate;
> +st->sample_aspect_ratio = ist->sample_aspect_ratio;
> +st->time_base = ist->time_base;
> +st->avg_frame_rate = ist->avg_frame_rate;
> #if FF_API_LAVF_AVCTX
> FF_DISABLE_DEPRECATION_WARNINGS
> -if (s->streams[i]->codecpar->codec_tag == MKTAG('t','m','c','d'))
> -st->codec->time_base = s->streams[i]->codec->time_base;
> +if (ipar->codec_tag == MKTAG('t','m','c','d'))
> +st->codec->time_base = ist->codec->time_base;
> FF_ENABLE_DEPRECATION_WARNINGS
> #endif
> -av_dict_copy(&st->metadata, s->streams[i]->metadata, 0);
> +av_dict_copy(&st->metadata, ist->metadata, 0);
> }
>
> return 0;
>
Will apply this patchset tomorrow unless there are objections.
- 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] avformat/mux: Remove unnecessary unreferencing of AVPacket
Andreas Rheinhardt:
> Since commit c5324d92c5f206dcdc2cf93ae237eaa7c1ad0a40 all custom
> interleave_packet() functions always return clean packets (even on
> error), so that unreferencing manually can be removed.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> libavformat/mux.c | 5 +
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/libavformat/mux.c b/libavformat/mux.c
> index f2de73af9b..c17686c0a6 100644
> --- a/libavformat/mux.c
> +++ b/libavformat/mux.c
> @@ -1075,10 +1075,7 @@ int ff_interleaved_peek(AVFormatContext *s, int stream,
> static int interleave_packet(AVFormatContext *s, AVPacket *out, AVPacket
> *in, int flush)
> {
> if (s->oformat->interleave_packet) {
> -int ret = s->oformat->interleave_packet(s, out, in, flush);
> -if (in)
> -av_packet_unref(in);
> -return ret;
> +return s->oformat->interleave_packet(s, out, in, flush);
> } else
> return ff_interleave_packet_per_dts(s, out, in, flush);
> }
>
Will apply this tomorrow unless there are objections.
- 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 2/3] avformat/matroskaenc: Remove pointless casts
by using a const void * pointer as an intermediate.
Signed-off-by: Andreas Rheinhardt
---
Why is the side-data API (both the packet as well as the stream one)
actually based around uint8_t * and not pointers to void despite
side-data being mostly structures and not just buffers?
libavformat/matroskaenc.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index f5968c17b4..9ad590cb93 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -836,7 +836,7 @@ static void mkv_write_video_color(AVIOContext *pb, const
AVStream *st,
uint8_t colour[(2 + 1 + 8) * 18 + (2 + 1) + 1];
AVIOContext buf, *dyn_cp = &buf;
int colorinfo_size;
-const uint8_t *side_data;
+const void *side_data;
ffio_init_context(dyn_cp, colour, sizeof(colour), 1, NULL, NULL, NULL,
NULL);
@@ -869,8 +869,7 @@ static void mkv_write_video_color(AVIOContext *pb, const
AVStream *st,
side_data = av_stream_get_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
NULL);
if (side_data) {
-const AVContentLightMetadata *metadata =
-(const AVContentLightMetadata*)side_data;
+const AVContentLightMetadata *metadata = side_data;
put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOCOLORMAXCLL, metadata->MaxCLL);
put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOCOLORMAXFALL,
metadata->MaxFALL);
}
@@ -880,8 +879,7 @@ static void mkv_write_video_color(AVIOContext *pb, const
AVStream *st,
if (side_data) {
ebml_master meta_element = start_ebml_master(
dyn_cp, MATROSKA_ID_VIDEOCOLORMASTERINGMETA, 10 * (2 + 1 + 8));
-const AVMasteringDisplayMetadata *metadata =
-(const AVMasteringDisplayMetadata*)side_data;
+const AVMasteringDisplayMetadata *metadata = side_data;
if (metadata->has_primaries) {
put_ebml_float(dyn_cp, MATROSKA_ID_VIDEOCOLOR_RX,
av_q2d(metadata->display_primaries[0][0]));
--
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 3/3] avformat/utils: Set stream side-data size even without side-data
Signed-off-by: Andreas Rheinhardt --- Was the earlier behaviour actually a bug (I think so) or is this new behaviour something that should be explicitly documented? Btw, the stream and packet side-data APIs differ in two more ways: av_packet_new_side_data() adds padding to the buffer and zeroes it whereas av_stream_new_side_data() does not. Should this be unified? libavformat/utils.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libavformat/utils.c b/libavformat/utils.c index e6158d8058..b12aff5eb0 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -5507,6 +5507,8 @@ uint8_t *av_stream_get_side_data(const AVStream *st, return st->side_data[i].data; } } +if (size) +*size = 0; return NULL; } -- 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 1/3] avformat/matroskaenc: Don't use stream side-data size
av_stream_get_side_data() tells the caller whether a stream has side
data of a specific type; if present it can also tell the caller the size
of the side data via an optional argument. The Matroska muxer always
used this optional argument, although it doesn't really need the size,
as the relevant side-data are not buffers, but structures. So change
this.
Furthermore, relying on the size also made the code susceptible to
a quirk of av_stream_get_side_data(): It only sets the size argument if
it found side data of the desired type. mkv_write_video_color() checks
for side-data twice with the same variable for the size without resetting
the size in between; if the second type of side-data isn't present, the
size will still be what it was after the first call. This was not
dangerous in practice, as the check for the existence of the second
side-data compared the size with the expected size, so it would only be
problematic if lots of elements were to be added to AVContentLightMetadata.
Signed-off-by: Andreas Rheinhardt
---
libavformat/matroskaenc.c | 19 ---
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
index fccfee539a..f5968c17b4 100644
--- a/libavformat/matroskaenc.c
+++ b/libavformat/matroskaenc.c
@@ -835,7 +835,6 @@ static void mkv_write_video_color(AVIOContext *pb, const
AVStream *st,
* plus another byte to stay clear of the end. */
uint8_t colour[(2 + 1 + 8) * 18 + (2 + 1) + 1];
AVIOContext buf, *dyn_cp = &buf;
-int side_data_size = 0;
int colorinfo_size;
const uint8_t *side_data;
@@ -868,8 +867,8 @@ static void mkv_write_video_color(AVIOContext *pb, const
AVStream *st,
}
side_data = av_stream_get_side_data(st, AV_PKT_DATA_CONTENT_LIGHT_LEVEL,
-&side_data_size);
-if (side_data_size) {
+NULL);
+if (side_data) {
const AVContentLightMetadata *metadata =
(const AVContentLightMetadata*)side_data;
put_ebml_uint(dyn_cp, MATROSKA_ID_VIDEOCOLORMAXCLL, metadata->MaxCLL);
@@ -877,8 +876,8 @@ static void mkv_write_video_color(AVIOContext *pb, const
AVStream *st,
}
side_data = av_stream_get_side_data(st,
AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
-&side_data_size);
-if (side_data_size == sizeof(AVMasteringDisplayMetadata)) {
+NULL);
+if (side_data) {
ebml_master meta_element = start_ebml_master(
dyn_cp, MATROSKA_ID_VIDEOCOLORMASTERINGMETA, 10 * (2 + 1 + 8));
const AVMasteringDisplayMetadata *metadata =
@@ -919,14 +918,13 @@ static void mkv_write_video_projection(AVFormatContext
*s, AVIOContext *pb,
const AVStream *st)
{
ebml_master projection;
-int side_data_size = 0;
uint8_t private[20];
const AVSphericalMapping *spherical =
(const AVSphericalMapping *)av_stream_get_side_data(st,
AV_PKT_DATA_SPHERICAL,
-&side_data_size);
+NULL);
-if (!side_data_size)
+if (!spherical)
return;
if (spherical->projection != AV_SPHERICAL_EQUIRECTANGULAR &&
@@ -1028,7 +1026,6 @@ static int mkv_write_stereo_mode(AVFormatContext *s,
AVIOContext *pb,
const AVDictionaryEntry *tag;
MatroskaVideoStereoModeType format = MATROSKA_VIDEO_STEREOMODE_TYPE_NB;
const AVStereo3D *stereo;
-int side_data_size = 0;
*h_width = 1;
*h_height = 1;
@@ -1052,8 +1049,8 @@ static int mkv_write_stereo_mode(AVFormatContext *s,
AVIOContext *pb,
}
stereo = (const AVStereo3D*)av_stream_get_side_data(st,
AV_PKT_DATA_STEREO3D,
-&side_data_size);
-if (side_data_size >= sizeof(AVStereo3D)) {
+NULL);
+if (stereo) {
switch (stereo->type) {
case AV_STEREO3D_2D:
format = MATROSKA_VIDEO_STEREOMODE_TYPE_MONO;
--
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 3/3] avformat/movenc: Add an automatic timescale computation
On Mon, 20 Apr 2020, Kevin Wheatley wrote:
Activated when the -mov_timescale command line/MOVMuxContext
parameter is set to 0 or less. If the user passes any value
greater than 0, then it will be used as-is. The default
value is kept unchanged at 1000.
When active, it uses the base assumption from the QuickTime
specification of 600 and combines the video stream time
bases to compute an accurate answer if possible.
Maybe I am wrong, but I would think that you should determine the MOV
timescale based on the track timescales, and not inspect time bases
directly.
I think an attempt should be made to calculate the least common multiple
of track timescales, but if it becomes too big, then do some other
heuristic, like select biggest track timescale?
Regards,
Marton
In cases when the first stream result falls outside
MOV_MAX_AUTO_TIMESCALE or if a non-integer video stream is
encounted, then the first stream's time_base will be used as the
base.
Signed-off-by: Kevin Wheatley
---
libavformat/movenc.c | 71
libavformat/movenc.h | 1 +
2 files changed, 72 insertions(+)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 508fa73..8edb848 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -6208,6 +6208,70 @@ static int
mov_create_dvd_sub_decoder_specific_info(MOVTrack *track,
return 0;
}
+static int mov_determine_movie_timescale(AVFormatContext *s, MOVMuxContext
*mov)
+{
+// Assume typical integer frame rates:
+// 600 is a common multiple of 24, 25, 30, 50, 60, etc.
+// see p221, https://developer.apple.com/standards/qtff-2001.pdf
+int timescale = 600;
+AVRational temp;
+int exact;
+int first_video_track = 1;
+
+// If the user specified a timescale, just use it as-is
+if (mov->mov_timescale > 0) {
+return mov->mov_timescale;
+}
+
+// Determine the timescale, based on the video stream time_base values
+for (int i = 0; i < s->nb_streams; i++) {
+AVStream *st = s->streams[i];
+if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
+
+// If the first video track is not an integer, use its denominator
+// as the basis of the remaining calculations
+if (first_video_track && st->time_base.num != 1) {
+timescale = st->time_base.den;
+av_log(s, AV_LOG_VERBOSE, "Using first video stream non-integer
time_base: %d/%d\n",
+ st->time_base.den, st->time_base.num);
+}
+
+// determine if we can make an even multiple of the current
timescale and the video track
+av_log(s, AV_LOG_VERBOSE, "Current estimated timescale: %d\n",
timescale);
+av_log(s, AV_LOG_TRACE, "Stream #%d time_base: %d/%d\n", i,
+ st->time_base.num, st->time_base.den);
+
+// Try keep the time_scale within a sensible bound
+exact = av_reduce(&temp.num, &temp.den,
+ (int64_t)timescale * st->time_base.num,
+ st->time_base.den,
+ MOV_MAX_AUTO_TIMESCALE);
+
+// Use a scaled version of the timescale
+if (exact) {
+timescale *= temp.den;
+av_log(s, AV_LOG_TRACE, "Adjusted timescale: %d/%d %d\n",
+ temp.den, temp.num, timescale);
+} else {
+// We overflowed try the denominator as is
+timescale = temp.den;
+av_log(s, AV_LOG_WARNING, "Timescale calculation out of bounds
approximating %d/%d %d\n",
+ temp.den, temp.num, timescale);
+}
+
+if (first_video_track && ((timescale > MOV_MAX_AUTO_TIMESCALE) ||
!exact)) {
+timescale = st->time_base.den;
+av_log(s, AV_LOG_WARNING, "Potentially large timescale, "
+ "using first video stream time_base:
%d/%d\n",
+ st->time_base.den, st->time_base.num);
+}
+first_video_track = 0;
+}
+}
+av_log(s, AV_LOG_VERBOSE, "Final timescale: %d\n", timescale);
+return timescale;
+}
+
static int mov_init(AVFormatContext *s)
{
MOVMuxContext *mov = s->priv_data;
@@ -6266,6 +6330,13 @@ static int mov_init(AVFormatContext *s)
mov->reserved_moov_size = -1;
}
+mov->mov_timescale = mov_determine_movie_timescale(s, mov);
+if (mov->mov_timescale > MOV_MAX_AUTO_TIMESCALE) {
+av_log(s, AV_LOG_WARNING, "Potentially large timescale %d, use "
+ "-mov_timescale if you have issues\n",
+ mov->mov_timescale);
+}
+
if (mov->use_editlist < 0) {
mov->use_editlist = 1;
if (mov->flags & FF_MOV_FLAG_FRAGMENT &&
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index 322968c..4d6b7b7 100644
--- a/
Re: [FFmpeg-devel] [PATCH 2/3] avformat/movenc: Use base container timescale, instead of hard coded default
On Mon, 20 Apr 2020, Kevin Wheatley wrote:
Signed-off-by: Kevin Wheatley
---
libavformat/movenc.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
You should squash this patch with the previous.
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 7d79eca..508fa73 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -2879,7 +2879,7 @@ static int mov_write_tkhd_tag(AVIOContext *pb,
MOVMuxContext *mov,
MOVTrack *track, AVStream *st)
{
int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
- MOV_TIMESCALE, track->timescale,
+ mov->mov_timescale, track->timescale,
AV_ROUND_UP);
int version = duration < INT32_MAX ? 0 : 1;
int flags = MOV_TKHD_FLAG_IN_MOVIE;
@@ -3027,7 +3027,7 @@ static int mov_write_edts_tag(AVIOContext *pb,
MOVMuxContext *mov,
MOVTrack *track)
{
int64_t duration = av_rescale_rnd(calc_pts_duration(mov, track),
- MOV_TIMESCALE, track->timescale,
+ mov->mov_timescale, track->timescale,
AV_ROUND_UP);
int version = duration < INT32_MAX ? 0 : 1;
int entry_size, entry_count, size;
@@ -3046,7 +3046,7 @@ static int mov_write_edts_tag(AVIOContext *pb,
MOVMuxContext *mov,
}
}
-delay = av_rescale_rnd(start_dts + start_ct, MOV_TIMESCALE,
+delay = av_rescale_rnd(start_dts + start_ct, mov->mov_timescale,
track->timescale, AV_ROUND_DOWN);
version |= delay < INT32_MAX ? 0 : 1;
@@ -3081,8 +3081,8 @@ static int mov_write_edts_tag(AVIOContext *pb,
MOVMuxContext *mov,
/* Avoid accidentally ending up with start_ct = -1 which has got a
* special meaning. Normally start_ct should end up positive or zero
* here, but use FFMIN in case dts is a small positive integer
- * rounded to 0 when represented in MOV_TIMESCALE units. */
-av_assert0(av_rescale_rnd(start_dts, MOV_TIMESCALE, track->timescale,
AV_ROUND_DOWN) <= 0);
+ * rounded to 0 when represented in mov->mov_timescale units. */
+av_assert0(av_rescale_rnd(start_dts, mov->mov_timescale,
track->timescale, AV_ROUND_DOWN) <= 0);
start_ct = -FFMIN(start_dts, 0);
/* Note, this delay is calculated from the pts of the first sample,
* ensuring that we don't reduce the duration for cases with
@@ -3316,7 +3316,7 @@ static int mov_write_mvhd_tag(AVIOContext *pb,
MOVMuxContext *mov)
if (mov->tracks[i].entry > 0 && mov->tracks[i].timescale) {
int64_t max_track_len_temp = av_rescale_rnd(
calc_pts_duration(mov,
&mov->tracks[i]),
-MOV_TIMESCALE,
+mov->mov_timescale,
mov->tracks[i].timescale,
AV_ROUND_UP);
if (max_track_len < max_track_len_temp)
@@ -3345,7 +3345,7 @@ static int mov_write_mvhd_tag(AVIOContext *pb,
MOVMuxContext *mov)
avio_wb32(pb, mov->time); /* creation time */
avio_wb32(pb, mov->time); /* modification time */
}
-avio_wb32(pb, MOV_TIMESCALE);
+avio_wb32(pb, mov->mov_timescale);
(version == 1) ? avio_wb64(pb, max_track_len) : avio_wb32(pb,
max_track_len); /* duration of longest track */
avio_wb32(pb, 0x0001); /* reserved (preferred rate) 1.0 = normal */
@@ -5921,7 +5921,7 @@ static int mov_create_chapter_track(AVFormatContext *s,
int tracknum)
track->mode = mov->mode;
track->tag = MKTAG('t','e','x','t');
-track->timescale = MOV_TIMESCALE;
+track->timescale = mov->mov_timescale;
track->par = avcodec_parameters_alloc();
if (!track->par)
return AVERROR(ENOMEM);
@@ -5982,8 +5982,8 @@ static int mov_create_chapter_track(AVFormatContext *s,
int tracknum)
AVChapter *c = s->chapters[i];
AVDictionaryEntry *t;
-int64_t end = av_rescale_q(c->end, c->time_base,
(AVRational){1,MOV_TIMESCALE});
-pkt.pts = pkt.dts = av_rescale_q(c->start, c->time_base,
(AVRational){1,MOV_TIMESCALE});
+int64_t end = av_rescale_q(c->end, c->time_base,
(AVRational){1,mov->mov_timescale});
+pkt.pts = pkt.dts = av_rescale_q(c->start, c->time_base,
(AVRational){1,mov->mov_timescale});
pkt.duration = end - pkt.dts;
if ((t = av_dict_get(c->metadata, "title", NULL, 0))) {
@@ -6518,7 +6518,7 @@ static int mov_init(AVFormatContext *s)
} else if (st->codecpar->codec_type == AVMEDIA_TYPE_DATA) {
track->timescale = st->time_base.den;
} else {
-track->timescale = MOV_TIMESCALE;
+track->timescale = mov->mov_timescale;
I think this sho
Re: [FFmpeg-devel] [PATCH 1/3] avformat/movenc: Add command line option to set base mov file timescale
On Mon, 20 Apr 2020, Kevin Wheatley wrote:
Signed-off-by: Kevin Wheatley
---
libavformat/movenc.c | 1 +
libavformat/movenc.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 253cff8..7d79eca 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -91,6 +91,7 @@ static const AVOption options[] = {
{ "min_frag_duration", "Minimum fragment duration", offsetof(MOVMuxContext,
min_fragment_duration), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
{ "frag_size", "Maximum fragment size", offsetof(MOVMuxContext,
max_fragment_size), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM},
{ "ism_lookahead", "Number of lookahead entries for ISM files",
offsetof(MOVMuxContext, ism_lookahead), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX,
AV_OPT_FLAG_ENCODING_PARAM},
+{ "mov_timescale", "set timescale of mov container",
offsetof(MOVMuxContext, mov_timescale), AV_OPT_TYPE_INT, {.i64 = MOV_TIMESCALE}, 0, INT_MAX,
AV_OPT_FLAG_ENCODING_PARAM},
Docs update is missing.
{ "video_track_timescale", "set timescale of all video tracks",
offsetof(MOVMuxContext, video_track_timescale), AV_OPT_TYPE_INT, {.i64 = 0}, 0, INT_MAX,
AV_OPT_FLAG_ENCODING_PARAM},
{ "brand","Override major brand", offsetof(MOVMuxContext, major_brand),
AV_OPT_TYPE_STRING, {.str = NULL}, .flags = AV_OPT_FLAG_ENCODING_PARAM },
{ "use_editlist", "use edit list", offsetof(MOVMuxContext, use_editlist),
AV_OPT_TYPE_BOOL, {.i64 = -1}, -1, 1, AV_OPT_FLAG_ENCODING_PARAM},
diff --git a/libavformat/movenc.h b/libavformat/movenc.h
index 997b2d6..322968c 100644
--- a/libavformat/movenc.h
+++ b/libavformat/movenc.h
@@ -205,6 +205,7 @@ typedef struct MOVMuxContext {
AVIOContext *mdat_buf;
int first_trun;
+int mov_timescale;
int video_track_timescale;
int reserved_moov_size; ///< 0 for disabled, -1 for automatic, size
otherwise
--
1.8.5.6
Regards,
Marton
___
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] libavformat/internal.h - Adjust MAX_URL_SIZE to 8k
On Sun, 10 May 2020, Joey Smith wrote: Updated patch attached Thanks, applied. Regards, Marton ___ 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/avformat: Remove redundant "NOT PART OF PUBLIC API"
Andreas Rheinhardt:
> AVStream.request_probe as well as AVStream.mux_ts_offset are below the
> separator of public and private fields, so that a further "NOT PART OF
> PUBLIC API" is redundant.
>
> Signed-off-by: Andreas Rheinhardt
> ---
> I initially also wanted to remove the internal note for AVStream's private
> fields ("be aware that physically removing these fields will break
> ABI"), but then I became aware that ffmpeg.c uses several of them
> (pts_wrap_bits, first_dts, cur_dts, codec_info_nb_frames). If I am not
> mistaken, this means that one can't even add new public fields in front
> of the public/private boundary in AVStream at all.
>
> libavformat/avformat.h | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 9b9b634ec3..f9291a26ec 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -1124,7 +1124,6 @@ typedef struct AVStream {
> * -1 -> probing finished
> * 0 -> no probing requested
> * rest -> perform probing with request_probe being the minimum score to
> accept.
> - * NOT PART OF PUBLIC API
> */
> int request_probe;
> /**
> @@ -1170,7 +1169,6 @@ typedef struct AVStream {
>
> /**
> * Timestamp offset added to timestamps before muxing
> - * NOT PART OF PUBLIC API
> */
> int64_t mux_ts_offset;
>
Will apply this tomorrow unless there are objections.
- 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]lavf/rtpdec_jpeg: Fix JFIF version
Hi! When receiving jpeg frames via rtp, libavformat currently writes a wrong JFIF version (2.01 instead of 1.02), see also 85ca012ba68. Attached patch fixes the version. Please comment, Carl Eugen From cf3250c1f044a940336ce17484a1d783849ed71f Mon Sep 17 00:00:00 2001 From: Carl Eugen Hoyos Date: Thu, 21 May 2020 22:03:08 +0200 Subject: [PATCH] lavf/rtpdec_jpeg: Fix JFIF version. See also 85ca012b --- libavformat/rtpdec_jpeg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libavformat/rtpdec_jpeg.c b/libavformat/rtpdec_jpeg.c index 931463cec4..b32d074136 100644 --- a/libavformat/rtpdec_jpeg.c +++ b/libavformat/rtpdec_jpeg.c @@ -112,7 +112,7 @@ static int jpeg_create_header(uint8_t *buf, int size, uint32_t type, uint32_t w, jpeg_put_marker(&pbc, APP0); bytestream2_put_be16(&pbc, 16); bytestream2_put_buffer(&pbc, "JFIF", 5); -bytestream2_put_be16(&pbc, 0x0201); +bytestream2_put_be16(&pbc, 0x0102); bytestream2_put_byte(&pbc, 0); bytestream2_put_be16(&pbc, 1); bytestream2_put_be16(&pbc, 1); -- 2.24.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] [RFC PATCH v2] libavcodec/jpeg2000_parser: Add jpeg2000 parser
Am Mi., 20. Mai 2020 um 22:21 Uhr schrieb Michael Niedermayer : > > On Wed, May 20, 2020 at 08:48:41PM +0530, Gautam Ramakrishnan wrote: > > On Tue, Apr 21, 2020 at 3:41 AM Michael Niedermayer > > wrote: > > > > > > On Mon, Apr 20, 2020 at 04:13:44PM +0530, Gautam Ramakrishnan wrote: > > > > On Mon, Apr 20, 2020 at 3:38 PM Michael Niedermayer > > > > wrote: > > > > > > > > > > On Mon, Apr 20, 2020 at 01:36:47AM +0530, gautamr...@gmail.com wrote: > > > > > > From: Gautam Ramakrishnan > > > > > > > > > > > > I have attempted to write a JPEG2000 Parser. Need > > > > > > help on testing the code and some tips on how to > > > > > > > > > > to test the code i would sugest to generate a file > > > > > or files with many jpeg2000 images and then try to > > > > > decode it to -f framecrc > > > > This helps me check whether the image is correct by comparing the CRC > > > > value? > > > > > if that work repeat while varying the packet size > > > > > input to the parser, a parser must work with anything > > > > > from 1 byte per input to sizes being larger than a > > > > > single frame. > > > > > > > > > So a packet to a parser is basically a smaller unit to which the parser > > > > is fed > > > > data to? When I tried printing buffer size during parse, it shows 4096. > > > > > > > Does that mean the packet size was 4096? > > > > > > yes, that likely comes from > > > libavformat/img2dec.c:size[0] = 4096; > > From my understanding of the documentation, the -packetsize option can > > change the > > value from 4096 to any particular I want right? However when I try setting > > the > > -packetsize option, the buf_size variable still shows up as 4096. > > img2dec hardcodes 4096, thats something you could change of course. > packetsize currently is listed with AV_OPT_FLAG_ENCODING_PARAM so its a muxer > option. Some demuxers seem to set packet_size though instead of using it as > input from the user > The header does not document packet_size with any detail > > Its probably best to add a new AVOption to img2dec to adjust the 4096 Can't the image2pipe demuxer option -frame_size be used instead? 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".
Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.
Am Mi., 20. Mai 2020 um 15:49 Uhr schrieb Martin Storsjö : > Would it work better [...] Or we could go for the simple solution which somehow became out-of-style here recently. 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".
Re: [FFmpeg-devel] [PATCH 2/2] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.
On Thu, May 21, 2020 at 12:37 AM Michael Niedermayer wrote: > gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0 > used with ccache on a x86-64 > Huh, I guess there's no early abort for conditionals in a preprocessor statement with __has_builtin for some reason. I've added a AV_HAS_BUILTIN macro to workaround this. - dale sat_math_builtin_v7.patch Description: Binary data ___ 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] fix typo in avformat/mov.c
avformat/mov: typo. when sidx time is found should assing it to pts instead
of dts
Fixes: AVPacket.dts was't monotonically increasing
---
libavformat/mov.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/mov.c b/libavformat/mov.c
index e11c9f4..b2132e6 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -4827,7 +4827,7 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb,
MOVAtom atom)
} else if (frag_stream_info->sidx_pts != AV_NOPTS_VALUE) {
// FIXME: sidx earliest_presentation_time is *PTS*, s.b.
// pts = frag_stream_info->sidx_pts;
-dts = frag_stream_info->sidx_pts - sc->time_offset;
+pts = frag_stream_info->sidx_pts - sc->time_offset;
av_log(c->fc, AV_LOG_DEBUG, "found sidx time %"PRId64
", using it for pts\n", pts);
} else if (frag_stream_info->tfdt_dts != AV_NOPTS_VALUE) {
--
2.6.4
___
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] [RFC PATCH v2] libavcodec/jpeg2000_parser: Add jpeg2000 parser
On Thu, May 21, 2020 at 1:51 AM Michael Niedermayer wrote: > > On Wed, May 20, 2020 at 08:48:41PM +0530, Gautam Ramakrishnan wrote: > > On Tue, Apr 21, 2020 at 3:41 AM Michael Niedermayer > > wrote: > > > > > > On Mon, Apr 20, 2020 at 04:13:44PM +0530, Gautam Ramakrishnan wrote: > > > > On Mon, Apr 20, 2020 at 3:38 PM Michael Niedermayer > > > > wrote: > > > > > > > > > > On Mon, Apr 20, 2020 at 01:36:47AM +0530, gautamr...@gmail.com wrote: > > > > > > From: Gautam Ramakrishnan > > > > > > > > > > > > I have attempted to write a JPEG2000 Parser. Need > > > > > > help on testing the code and some tips on how to > > > > > > > > > > to test the code i would sugest to generate a file > > > > > or files with many jpeg2000 images and then try to > > > > > decode it to -f framecrc > > > > This helps me check whether the image is correct by comparing the CRC > > > > value? > > > > > if that work repeat while varying the packet size > > > > > input to the parser, a parser must work with anything > > > > > from 1 byte per input to sizes being larger than a > > > > > single frame. > > > > > > > > > So a packet to a parser is basically a smaller unit to which the parser > > > > is fed > > > > data to? When I tried printing buffer size during parse, it shows 4096. > > > > > > > Does that mean the packet size was 4096? > > > > > > yes, that likely comes from > > > libavformat/img2dec.c:size[0] = 4096; > > From my understanding of the documentation, the -packetsize option can > > change the > > value from 4096 to any particular I want right? However when I try setting > > the > > -packetsize option, the buf_size variable still shows up as 4096. > > img2dec hardcodes 4096, thats something you could change of course. > packetsize currently is listed with AV_OPT_FLAG_ENCODING_PARAM so its a muxer > option. Some demuxers seem to set packet_size though instead of using it as > input from the user > The header does not document packet_size with any detail > > Its probably best to add a new AVOption to img2dec to adjust the 4096 This would make debugging easy in general. Probably should start with this > > also improving the documentation is certainly not a bad idea > > Thanks > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > What does censorship reveal? It reveals fear. -- Julian Assange > ___ > 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". -- - Gautam | ___ 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] Specifying MOV_TIMESCALE from a command-line switch
>> Has anybody proposed a patch to allow people to set that MOV_TIMESCALE >> variable from the command line? >> It seems that would be an easy patch to do and could be used to fix this and >> numerous other requests for different movie timescales. >> I looked for discussion on this and didn't find mention in bugs, but found >> this thread: >> https://urldefense.proofpoint.com/v2/url?u=http-3A__www.ffmpeg-2Darchive.org_Quicktime-2DSpecify-2Da-2DMovie-2DHeader-2Dtimescale-2Dof-2D24000-2Dtt4683020.html-23none&d=DwIGaQ&c=gFTBenQ7Vj71sUi1A4CkFnmPzqwDo07QsHw-JRepxyw&r=i_WAJaOy8vrtjOFMFjd3LWqJsLBEpPDcwb8xZYhypuo&m=_30m0iw_DJYUOid66LGZjqDwjcq60rEYogk5aDA2We4&s=KPX1Vh0uvrJKlna6CKexnb3eViSgjz3R186djFemY1Q&e= >> >> ..not sure if this has been discussed elsewhere and I missed it. >> I'd be willing to do the work if the team agrees this is a good idea, or if >> there is another proposal that would have the same result. >https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ffmpeg.org_project_ffmpeg_list_-3Fseries-3D975&d=DwIGaQ&c=gFTBenQ7Vj71sUi1A4CkFnmPzqwDo07QsHw-JRepxyw&r=i_WAJaOy8vrtjOFMFjd3LWqJsLBEpPDcwb8xZYhypuo&m=_30m0iw_DJYUOid66LGZjqDwjcq60rEYogk5aDA2We4&s=IgzDJaL9Apxmn9n9_TQctHb88KfG4Y15ax0oq5m7XPc&e= > >There is even some code which automatically determines time scale, but it >seems to limit it to 1? Shouldn't we simply set the MOV timescale as >the least common multiple of the track timescales? >Regards, >Marton Thanks for the response, Marton. The LCM of track timescales would work for our purposes, or one of the patches you referenced that Kevin Wheatley proposed. I'm not sure of the patch process. Is this a current work in progress? Any way I can help it to get to release? Thanks, John ___ 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]examples/hw_decode: Add binary mode to fopen() for non-POSIX systems
Hi!
Attached patch is supposed to fix ticket #8638.
Please comment, Carl Eugen
From 5349586a4c17727eafd770279e6768805fbb72a5 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos
Date: Thu, 21 May 2020 17:40:18 +0200
Subject: [PATCH] examples/hw_decode: Add binary mode to fopen() for non-POSIX
systems.
---
doc/examples/hw_decode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/doc/examples/hw_decode.c b/doc/examples/hw_decode.c
index f3286f472d..71be6e6709 100644
--- a/doc/examples/hw_decode.c
+++ b/doc/examples/hw_decode.c
@@ -223,7 +223,7 @@ int main(int argc, char *argv[])
}
/* open the file to dump raw data */
-output_file = fopen(argv[3], "w+");
+output_file = fopen(argv[3], "w+b");
/* actual decoding and dump the raw data */
while (ret >= 0) {
--
2.24.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 1/2] avfilter/vf_lut3d: initial float pixel format support
LGTM On 5/19/20, mindm...@gmail.com wrote: > From: Mark Reid > > --- > libavfilter/vf_lut3d.c | 207 +++-- > 1 file changed, 201 insertions(+), 6 deletions(-) > ___ 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/decode: remove ff_decode_bsfs_uninit()
On 5/20/2020 9:34 PM, myp...@gmail.com wrote:
> On Wed, May 20, 2020 at 12:08 PM James Almer wrote:
>>
>> It's been a wrapper for a simple av_bsf_free() call since c96904f525.
>>
>> Signed-off-by: James Almer
>> ---
>> libavcodec/decode.c | 7 +--
>> libavcodec/decode.h | 2 --
>> libavcodec/utils.c | 4 ++--
>> 3 files changed, 3 insertions(+), 10 deletions(-)
>>
>> diff --git a/libavcodec/decode.c b/libavcodec/decode.c
>> index 48a61d5419..f3327d74af 100644
>> --- a/libavcodec/decode.c
>> +++ b/libavcodec/decode.c
>> @@ -231,7 +231,7 @@ int ff_decode_bsfs_init(AVCodecContext *avctx)
>>
>> return 0;
>> fail:
>> -ff_decode_bsfs_uninit(avctx);
>> +av_bsf_free(&avci->bsf);
>> return ret;
>> }
>>
>> @@ -2005,8 +2005,3 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
>> if (!avctx->refcounted_frames)
>> av_frame_unref(avci->to_free);
>> }
>> -
>> -void ff_decode_bsfs_uninit(AVCodecContext *avctx)
>> -{
>> -av_bsf_free(&avctx->internal->bsf);
>> -}
>> diff --git a/libavcodec/decode.h b/libavcodec/decode.h
>> index c3e0e82f4c..0d69294def 100644
>> --- a/libavcodec/decode.h
>> +++ b/libavcodec/decode.h
>> @@ -66,8 +66,6 @@ int ff_decode_get_packet(AVCodecContext *avctx, AVPacket
>> *pkt);
>>
>> int ff_decode_bsfs_init(AVCodecContext *avctx);
> I think we better add a comment in the header if need to pairs using
> ff_decode_bsfs_init/av_bsf_free
Added a comment to ff_decode_bsfs_init() and pushed.
___
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/3] avcodec: deprecate Lossless and Intra Only encoder capabilites
On 5/21/2020 10:21 AM, Anton Khirnov wrote: > Quoting James Almer (2020-05-18 18:33:26) >> Both are codec properties and not encoder capabilities. The relevant >> AVCodecDescriptor.props flags exist for this purpose. >> >> Signed-off-by: James Almer >> --- > > Don't know if it makes sense to bump version just because something was > deprecated. > > Other than that looks ok. It's mostly so i have a version to reference in APIChanges. Pushed, thanks. ___ 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 v3 2/5] avfilter/vf_libopencv: add opencv HaarCascade classifier simple face detection filter
I kindly ask everybody including you to stop wasting time on
supporting only old opencv versions features, and instead to use such
resources in writing C++ filter wrapper with same name which will work
with latest opencv version.
Consider this module/file deprecated.
On 5/21/20, lance.lmw...@gmail.com wrote:
> On Thu, May 21, 2020 at 04:44:26PM +0200, Paul B Mahol wrote:
>> On 5/21/20, lance.lmw...@gmail.com wrote:
>> > On Thu, May 21, 2020 at 04:12:56PM +0200, Paul B Mahol wrote:
>> >> On 5/18/20, lance.lmw...@gmail.com wrote:
>> >> > On Mon, May 18, 2020 at 01:11:12PM +0200, Paul B Mahol wrote:
>> >> >> This opencv module is obsolete for latest opencv.
>> >> >>
>> >> >> Instead there should be C++ wrapper.
>> >> >
>> >> > Sorry, where is the C++ wrapper? Now Opencv 2.x and 3.x is good
>> >> > to use for my current function. By my initial testing, the
>> >> > performance
>> >> > is
>> >> > very good, we can easily to get realtime for HD, the drawbacks are:
>> >> > 1. The face is not directly opposite and cannot be recognized.
>> >> > 2. there are some false detect for video.
>> >> >
>> >> > If have any old discussion for that, please give the discussion link
>> >> > for
>> >> > reference. I'm not keep track of the ML before.
>> >>
>> >> Nope, i'm not doing others people work for free.
>> >
>> > So are you against continuing to develop existing opencv features？Now I
>> > have tested with opencv 2.10 and 3.10, 3.13, but > 4.0 will not working
>> > as
>> > expected for known issue.
>>
>> Yes, and I want that known issue with latest opencv to be properly
>> fixed, instead
>> of using shortcut hacks that will work with old versions only.
>
> The question is without my patchset, the current opencv didn't work with the
> latest opencv still, it's not new issue. Or we should mark the opencv
> filter
> deprecated and remove it, or someone write a c++ wrapper to replace it.
>
> Also the current function haven't depend on the latest opencv version yet.
>
>>
>> >
>> >>
>> >> >
>> >> >
>> >> >>
>> >> >> On 5/18/20, lance.lmw...@gmail.com wrote:
>> >> >> > From: Limin Wang
>> >> >> >
>> >> >> > Signed-off-by: Limin Wang
>> >> >> > ---
>> >> >> > change the update_metadata() to postprocess() only, I'll add
>> >> >> > opencv
>> >> >> > drawbox
>> >> >> > filter and it need preprocess() to get the meta, so I prefer to
>> >> >> > change
>> >> >> > the
>> >> >> > function name for better readablity, in future, it may have other
>> >> >> > processing
>> >> >> > than metadata only.
>> >> >> >
>> >> >> > configure | 1 +
>> >> >> > doc/filters.texi | 29 +++
>> >> >> > libavfilter/vf_libopencv.c | 164
>> >> >> > -
>> >> >> > 3 files changed, 191 insertions(+), 3 deletions(-)
>> >> >> >
>> >> >> > diff --git a/configure b/configure
>> >> >> > index 34afdaad28..281b67efc4 100755
>> >> >> > --- a/configure
>> >> >> > +++ b/configure
>> >> >> > @@ -2123,6 +2123,7 @@ HEADERS_LIST="
>> >> >> > machine_ioctl_meteor_h
>> >> >> > malloc_h
>> >> >> > opencv2_core_core_c_h
>> >> >> > +opencv2_objdetect_objdetect_c_h
>> >> >> > OpenGL_gl3_h
>> >> >> > poll_h
>> >> >> > sys_param_h
>> >> >> > diff --git a/doc/filters.texi b/doc/filters.texi
>> >> >> > index d9ba0fffa1..f938dd04de 100644
>> >> >> > --- a/doc/filters.texi
>> >> >> > +++ b/doc/filters.texi
>> >> >> > @@ -14177,6 +14177,35 @@ other parameters is 0.
>> >> >> > These parameters correspond to the parameters assigned to the
>> >> >> > libopencv function @code{cvSmooth}.
>> >> >> >
>> >> >> > +@subsection facedetect
>> >> >> > +Face detection using Haar Feature-based Cascade Classifiers.
>> >> >> > +
>> >> >> > +The filter takes the following parameters:
>> >> >> > +@var{xml_model}|@var{qoffset}.
>> >> >> > +
>> >> >> > +@var{xml_model} is the path of pre-trained classifiers, The C API
>> >> >> > still
>> >> >> > +does not support the newer cascade format, please use the old
>> >> >> > format
>> >> >> > +haarcascade_frontalface_alt.xml which type_id is
>> >> >> > opencv-haar-classifier.
>> >> >> > +
>> >> >> > +@var{qoffset}
>> >> >> > +If you want export the detected faces by ROI side data in frame,
>> >> >> > please
>> >> >> > set
>> >> >> > the
>> >> >> > +parameters, See also the @ref{addroi} filter. The range of
>> >> >> > qoffset
>> >> >> > is
>> >> >> > from
>> >> >> > [-1.0, 1.0]
>> >> >> > +
>> >> >> > +By default the filter will report these metadata values if face
>> >> >> > are
>> >> >> > +detected:
>> >> >> > +@table @option
>> >> >> > +@item lavfi.facedetect.nb_faces
>> >> >> > +Display the detected face number
>> >> >> > +
>> >> >> > +@item lavfi.facedetect.face_id.x, lavfi.facedetect.face_id.y
>> >> >> > +Display x and y of every faces, face_id is the face index which
>> >> >> > is
>> >> >> > range
>> >> >> > +from [0, nb_faces-1]
>> >> >> > +
>> >> >> > +@item lavfi.facedetect.face_id.w, lavfi.facedetect.face_id.h
>> >> >> > +Display width and height
Re: [FFmpeg-devel] [PATCH 2/2] avfilter/vf_lut3d: prelut support for 3d cinespace luts
Probably ok, except code style. Please keep code style consistent across files. On 5/19/20, mindm...@gmail.com wrote: > From: Mark Reid > > --- > libavfilter/vf_lut3d.c | 367 +++-- > 1 file changed, 312 insertions(+), 55 deletions(-) > ___ 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 v3 2/5] avfilter/vf_libopencv: add opencv HaarCascade classifier simple face detection filter
On Thu, May 21, 2020 at 04:44:26PM +0200, Paul B Mahol wrote:
> On 5/21/20, lance.lmw...@gmail.com wrote:
> > On Thu, May 21, 2020 at 04:12:56PM +0200, Paul B Mahol wrote:
> >> On 5/18/20, lance.lmw...@gmail.com wrote:
> >> > On Mon, May 18, 2020 at 01:11:12PM +0200, Paul B Mahol wrote:
> >> >> This opencv module is obsolete for latest opencv.
> >> >>
> >> >> Instead there should be C++ wrapper.
> >> >
> >> > Sorry, where is the C++ wrapper? Now Opencv 2.x and 3.x is good
> >> > to use for my current function. By my initial testing, the performance
> >> > is
> >> > very good, we can easily to get realtime for HD, the drawbacks are:
> >> > 1. The face is not directly opposite and cannot be recognized.
> >> > 2. there are some false detect for video.
> >> >
> >> > If have any old discussion for that, please give the discussion link for
> >> > reference. I'm not keep track of the ML before.
> >>
> >> Nope, i'm not doing others people work for free.
> >
> > So are you against continuing to develop existing opencv features？Now I
> > have tested with opencv 2.10 and 3.10, 3.13, but > 4.0 will not working as
> > expected for known issue.
>
> Yes, and I want that known issue with latest opencv to be properly
> fixed, instead
> of using shortcut hacks that will work with old versions only.
The question is without my patchset, the current opencv didn't work with the
latest opencv still, it's not new issue. Or we should mark the opencv filter
deprecated and remove it, or someone write a c++ wrapper to replace it.
Also the current function haven't depend on the latest opencv version yet.
>
> >
> >>
> >> >
> >> >
> >> >>
> >> >> On 5/18/20, lance.lmw...@gmail.com wrote:
> >> >> > From: Limin Wang
> >> >> >
> >> >> > Signed-off-by: Limin Wang
> >> >> > ---
> >> >> > change the update_metadata() to postprocess() only, I'll add opencv
> >> >> > drawbox
> >> >> > filter and it need preprocess() to get the meta, so I prefer to
> >> >> > change
> >> >> > the
> >> >> > function name for better readablity, in future, it may have other
> >> >> > processing
> >> >> > than metadata only.
> >> >> >
> >> >> > configure | 1 +
> >> >> > doc/filters.texi | 29 +++
> >> >> > libavfilter/vf_libopencv.c | 164
> >> >> > -
> >> >> > 3 files changed, 191 insertions(+), 3 deletions(-)
> >> >> >
> >> >> > diff --git a/configure b/configure
> >> >> > index 34afdaad28..281b67efc4 100755
> >> >> > --- a/configure
> >> >> > +++ b/configure
> >> >> > @@ -2123,6 +2123,7 @@ HEADERS_LIST="
> >> >> > machine_ioctl_meteor_h
> >> >> > malloc_h
> >> >> > opencv2_core_core_c_h
> >> >> > +opencv2_objdetect_objdetect_c_h
> >> >> > OpenGL_gl3_h
> >> >> > poll_h
> >> >> > sys_param_h
> >> >> > diff --git a/doc/filters.texi b/doc/filters.texi
> >> >> > index d9ba0fffa1..f938dd04de 100644
> >> >> > --- a/doc/filters.texi
> >> >> > +++ b/doc/filters.texi
> >> >> > @@ -14177,6 +14177,35 @@ other parameters is 0.
> >> >> > These parameters correspond to the parameters assigned to the
> >> >> > libopencv function @code{cvSmooth}.
> >> >> >
> >> >> > +@subsection facedetect
> >> >> > +Face detection using Haar Feature-based Cascade Classifiers.
> >> >> > +
> >> >> > +The filter takes the following parameters:
> >> >> > +@var{xml_model}|@var{qoffset}.
> >> >> > +
> >> >> > +@var{xml_model} is the path of pre-trained classifiers, The C API
> >> >> > still
> >> >> > +does not support the newer cascade format, please use the old format
> >> >> > +haarcascade_frontalface_alt.xml which type_id is
> >> >> > opencv-haar-classifier.
> >> >> > +
> >> >> > +@var{qoffset}
> >> >> > +If you want export the detected faces by ROI side data in frame,
> >> >> > please
> >> >> > set
> >> >> > the
> >> >> > +parameters, See also the @ref{addroi} filter. The range of qoffset
> >> >> > is
> >> >> > from
> >> >> > [-1.0, 1.0]
> >> >> > +
> >> >> > +By default the filter will report these metadata values if face are
> >> >> > +detected:
> >> >> > +@table @option
> >> >> > +@item lavfi.facedetect.nb_faces
> >> >> > +Display the detected face number
> >> >> > +
> >> >> > +@item lavfi.facedetect.face_id.x, lavfi.facedetect.face_id.y
> >> >> > +Display x and y of every faces, face_id is the face index which is
> >> >> > range
> >> >> > +from [0, nb_faces-1]
> >> >> > +
> >> >> > +@item lavfi.facedetect.face_id.w, lavfi.facedetect.face_id.h
> >> >> > +Display width and height of every faces, face_id is the face index
> >> >> > +which is range from [0, nb_faces-1]
> >> >> > +@end table
> >> >> > +
> >> >> > @section oscilloscope
> >> >> >
> >> >> > 2D Video Oscilloscope.
> >> >> > diff --git a/libavfilter/vf_libopencv.c b/libavfilter/vf_libopencv.c
> >> >> > index 8128030b8c..b2d19bb241 100644
> >> >> > --- a/libavfilter/vf_libopencv.c
> >> >> > +++ b/libavfilter/vf_libopencv.c
> >> >> > @@ -1,5 +1,6 @@
> >> >> > /*
> >> >> > * Copyright (c) 2010 Stefano Sabatini
> >> >>
Re: [FFmpeg-devel] [PATCH] avutil/video_enc_param: fix warning
Anton Khirnov (12020-05-21):
> This is wrong. We should not fix warnings, we should fix bugs. Warnings
> suggest there may be a bug, but not all warnings are correct.
>
> In this case, I believe the warnings is invalid and there is no problem
> to fix. It's correct that the comparison is always false on some
> platforms, but AFAIK no standard we adhere to guarantees that on all
> platforms.
Yet, we should strive to silence this warning, because invalid warnings
distract from valid ones.
It is tricky to do elegantly.
Maybe:
static inline int
check_overflow(size_t n, size_t s, size_t c)
{
return n <= SIZE_MAX / s && n * s < SIZE_MAX - c;
}
It would avoid the warning because n is size_t instead of unsigned.
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] avutil/video_enc_param: fix warning
> [FFmpeg-devel] [PATCH] avutil/video_enc_param: fix warning This is wrong. We should not fix warnings, we should fix bugs. Warnings suggest there may be a bug, but not all warnings are correct. In this case, I believe the warnings is invalid and there is no problem to fix. It's correct that the comparison is always false on some platforms, but AFAIK no standard we adhere to guarantees that on all platforms. -- Anton Khirnov ___ 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] avutil/video_enc_param: fix warning
On Thu, May 21, 2020 at 04:34:57PM +0200, Carl Eugen Hoyos wrote: > Am Do., 21. Mai 2020 um 16:21 Uhr schrieb : > > > > From: Limin Wang > > > > warning: comparison is always false due to limited range of data type > > [-Wtype-limits] > > Also nb_blocks is unsigned int, so nb_blocks * sizeof(AVVideoBlockParams) > > may overflow, > > so force to size_t > > > > Signed-off-by: Limin Wang > > --- > > libavutil/video_enc_params.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c > > index c46c0f1..4a4c85f 100644 > > --- a/libavutil/video_enc_params.c > > +++ b/libavutil/video_enc_params.c > > @@ -33,8 +33,8 @@ AVVideoEncParams *av_video_enc_params_alloc(enum > > AVVideoEncParamsType type, > > size_t size; > > > > size = sizeof(*par); > > -if (nb_blocks > SIZE_MAX / sizeof(AVVideoBlockParams) || > > -nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) > > +if (nb_blocks > UINT_MAX / sizeof(AVVideoBlockParams) || > > +(size_t)nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) > > I don't think this patch is ok. > > Shouldn't this be an assert() about sizeof(AVVideoBlockParams) instead? Sorry, I try to fix the warning message, so go ahead if anyone have a proper fix. > > 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". -- 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] avutil/video_enc_param: fix warning
lance.lmw...@gmail.com (12020-05-21): > I'm not sure, Mac compile give below warning: > > warning: comparison of constant 922337203685477580 with expression of type > 'unsigned int' is always false > [-Wtautological-constant-out-of-range-compare] > Certainly. And? > > The cast is unnecessary due to C's promotion rules. > Yes, It seems it's not necessary as the first test. The cast is unnecessary, period. Do you know C's promotion rules in this case? What I am really asking, in both cases, is: Do you understand what you are doing? 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] avutil/video_enc_param: fix warning
On Thu, May 21, 2020 at 04:34:39PM +0200, Nicolas George wrote: > lance.lmw...@gmail.com (12020-05-21): > > From: Limin Wang > > > > warning: comparison is always false due to limited range of data type > > [-Wtype-limits] > > > Also nb_blocks is unsigned int, so nb_blocks * sizeof(AVVideoBlockParams) > > may overflow, > > so force to size_t > > No it may not, the test just before prevents it. > > > > > Signed-off-by: Limin Wang > > --- > > libavutil/video_enc_params.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c > > index c46c0f1..4a4c85f 100644 > > --- a/libavutil/video_enc_params.c > > +++ b/libavutil/video_enc_params.c > > @@ -33,8 +33,8 @@ AVVideoEncParams *av_video_enc_params_alloc(enum > > AVVideoEncParamsType type, > > size_t size; > > > > size = sizeof(*par); > > > -if (nb_blocks > SIZE_MAX / sizeof(AVVideoBlockParams) || > > +if (nb_blocks > UINT_MAX / sizeof(AVVideoBlockParams) || > > These tests are not equivalent. I'm not sure, Mac compile give below warning: warning: comparison of constant 922337203685477580 with expression of type 'unsigned int' is always false [-Wtautological-constant-out-of-range-compare] > > > -nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) > > +(size_t)nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) > > The cast is unnecessary due to C's promotion rules. Yes, It seems it's not necessary as the first test. > > > return NULL; > > size += sizeof(AVVideoBlockParams) * nb_blocks; > > > > Regards, > > -- > Nicolas George -- 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 v3 2/5] avfilter/vf_libopencv: add opencv HaarCascade classifier simple face detection filter
On 5/21/20, lance.lmw...@gmail.com wrote:
> On Thu, May 21, 2020 at 04:12:56PM +0200, Paul B Mahol wrote:
>> On 5/18/20, lance.lmw...@gmail.com wrote:
>> > On Mon, May 18, 2020 at 01:11:12PM +0200, Paul B Mahol wrote:
>> >> This opencv module is obsolete for latest opencv.
>> >>
>> >> Instead there should be C++ wrapper.
>> >
>> > Sorry, where is the C++ wrapper? Now Opencv 2.x and 3.x is good
>> > to use for my current function. By my initial testing, the performance
>> > is
>> > very good, we can easily to get realtime for HD, the drawbacks are:
>> > 1. The face is not directly opposite and cannot be recognized.
>> > 2. there are some false detect for video.
>> >
>> > If have any old discussion for that, please give the discussion link for
>> > reference. I'm not keep track of the ML before.
>>
>> Nope, i'm not doing others people work for free.
>
> So are you against continuing to develop existing opencv features？Now I
> have tested with opencv 2.10 and 3.10, 3.13, but > 4.0 will not working as
> expected for known issue.
Yes, and I want that known issue with latest opencv to be properly
fixed, instead
of using shortcut hacks that will work with old versions only.
>
>>
>> >
>> >
>> >>
>> >> On 5/18/20, lance.lmw...@gmail.com wrote:
>> >> > From: Limin Wang
>> >> >
>> >> > Signed-off-by: Limin Wang
>> >> > ---
>> >> > change the update_metadata() to postprocess() only, I'll add opencv
>> >> > drawbox
>> >> > filter and it need preprocess() to get the meta, so I prefer to
>> >> > change
>> >> > the
>> >> > function name for better readablity, in future, it may have other
>> >> > processing
>> >> > than metadata only.
>> >> >
>> >> > configure | 1 +
>> >> > doc/filters.texi | 29 +++
>> >> > libavfilter/vf_libopencv.c | 164
>> >> > -
>> >> > 3 files changed, 191 insertions(+), 3 deletions(-)
>> >> >
>> >> > diff --git a/configure b/configure
>> >> > index 34afdaad28..281b67efc4 100755
>> >> > --- a/configure
>> >> > +++ b/configure
>> >> > @@ -2123,6 +2123,7 @@ HEADERS_LIST="
>> >> > machine_ioctl_meteor_h
>> >> > malloc_h
>> >> > opencv2_core_core_c_h
>> >> > +opencv2_objdetect_objdetect_c_h
>> >> > OpenGL_gl3_h
>> >> > poll_h
>> >> > sys_param_h
>> >> > diff --git a/doc/filters.texi b/doc/filters.texi
>> >> > index d9ba0fffa1..f938dd04de 100644
>> >> > --- a/doc/filters.texi
>> >> > +++ b/doc/filters.texi
>> >> > @@ -14177,6 +14177,35 @@ other parameters is 0.
>> >> > These parameters correspond to the parameters assigned to the
>> >> > libopencv function @code{cvSmooth}.
>> >> >
>> >> > +@subsection facedetect
>> >> > +Face detection using Haar Feature-based Cascade Classifiers.
>> >> > +
>> >> > +The filter takes the following parameters:
>> >> > +@var{xml_model}|@var{qoffset}.
>> >> > +
>> >> > +@var{xml_model} is the path of pre-trained classifiers, The C API
>> >> > still
>> >> > +does not support the newer cascade format, please use the old format
>> >> > +haarcascade_frontalface_alt.xml which type_id is
>> >> > opencv-haar-classifier.
>> >> > +
>> >> > +@var{qoffset}
>> >> > +If you want export the detected faces by ROI side data in frame,
>> >> > please
>> >> > set
>> >> > the
>> >> > +parameters, See also the @ref{addroi} filter. The range of qoffset
>> >> > is
>> >> > from
>> >> > [-1.0, 1.0]
>> >> > +
>> >> > +By default the filter will report these metadata values if face are
>> >> > +detected:
>> >> > +@table @option
>> >> > +@item lavfi.facedetect.nb_faces
>> >> > +Display the detected face number
>> >> > +
>> >> > +@item lavfi.facedetect.face_id.x, lavfi.facedetect.face_id.y
>> >> > +Display x and y of every faces, face_id is the face index which is
>> >> > range
>> >> > +from [0, nb_faces-1]
>> >> > +
>> >> > +@item lavfi.facedetect.face_id.w, lavfi.facedetect.face_id.h
>> >> > +Display width and height of every faces, face_id is the face index
>> >> > +which is range from [0, nb_faces-1]
>> >> > +@end table
>> >> > +
>> >> > @section oscilloscope
>> >> >
>> >> > 2D Video Oscilloscope.
>> >> > diff --git a/libavfilter/vf_libopencv.c b/libavfilter/vf_libopencv.c
>> >> > index 8128030b8c..b2d19bb241 100644
>> >> > --- a/libavfilter/vf_libopencv.c
>> >> > +++ b/libavfilter/vf_libopencv.c
>> >> > @@ -1,5 +1,6 @@
>> >> > /*
>> >> > * Copyright (c) 2010 Stefano Sabatini
>> >> > + * Copyright (c) 2020 Limin Wang
>> >> > *
>> >> > * This file is part of FFmpeg.
>> >> > *
>> >> > @@ -27,10 +28,16 @@
>> >> > #if HAVE_OPENCV2_CORE_CORE_C_H
>> >> > #include
>> >> > #include
>> >> > +#if HAVE_OPENCV2_OBJECTDETECT_OBJECTDETECT_C_H
>> >> > +#include
>> >> > +#else
>> >> > +#include
>> >> > +#endif
>> >> > #else
>> >> > #include
>> >> > #include
>> >> > #endif
>> >> > +
>> >> > #include "libavutil/avstring.h"
>> >> > #include "libavutil/common.h"
>> >> > #include "libavutil/file.h"
>> >> > @@ -82,6 +89,7 @@ typedef struct OCVContext {
>> >> > int (*init)(AVFi
Re: [FFmpeg-devel] [PATCH v3 2/5] avfilter/vf_libopencv: add opencv HaarCascade classifier simple face detection filter
On Thu, May 21, 2020 at 04:12:56PM +0200, Paul B Mahol wrote:
> On 5/18/20, lance.lmw...@gmail.com wrote:
> > On Mon, May 18, 2020 at 01:11:12PM +0200, Paul B Mahol wrote:
> >> This opencv module is obsolete for latest opencv.
> >>
> >> Instead there should be C++ wrapper.
> >
> > Sorry, where is the C++ wrapper? Now Opencv 2.x and 3.x is good
> > to use for my current function. By my initial testing, the performance is
> > very good, we can easily to get realtime for HD, the drawbacks are:
> > 1. The face is not directly opposite and cannot be recognized.
> > 2. there are some false detect for video.
> >
> > If have any old discussion for that, please give the discussion link for
> > reference. I'm not keep track of the ML before.
>
> Nope, i'm not doing others people work for free.
So are you against continuing to develop existing opencv features？Now I
have tested with opencv 2.10 and 3.10, 3.13, but > 4.0 will not working as
expected for known issue.
>
> >
> >
> >>
> >> On 5/18/20, lance.lmw...@gmail.com wrote:
> >> > From: Limin Wang
> >> >
> >> > Signed-off-by: Limin Wang
> >> > ---
> >> > change the update_metadata() to postprocess() only, I'll add opencv
> >> > drawbox
> >> > filter and it need preprocess() to get the meta, so I prefer to change
> >> > the
> >> > function name for better readablity, in future, it may have other
> >> > processing
> >> > than metadata only.
> >> >
> >> > configure | 1 +
> >> > doc/filters.texi | 29 +++
> >> > libavfilter/vf_libopencv.c | 164 -
> >> > 3 files changed, 191 insertions(+), 3 deletions(-)
> >> >
> >> > diff --git a/configure b/configure
> >> > index 34afdaad28..281b67efc4 100755
> >> > --- a/configure
> >> > +++ b/configure
> >> > @@ -2123,6 +2123,7 @@ HEADERS_LIST="
> >> > machine_ioctl_meteor_h
> >> > malloc_h
> >> > opencv2_core_core_c_h
> >> > +opencv2_objdetect_objdetect_c_h
> >> > OpenGL_gl3_h
> >> > poll_h
> >> > sys_param_h
> >> > diff --git a/doc/filters.texi b/doc/filters.texi
> >> > index d9ba0fffa1..f938dd04de 100644
> >> > --- a/doc/filters.texi
> >> > +++ b/doc/filters.texi
> >> > @@ -14177,6 +14177,35 @@ other parameters is 0.
> >> > These parameters correspond to the parameters assigned to the
> >> > libopencv function @code{cvSmooth}.
> >> >
> >> > +@subsection facedetect
> >> > +Face detection using Haar Feature-based Cascade Classifiers.
> >> > +
> >> > +The filter takes the following parameters:
> >> > +@var{xml_model}|@var{qoffset}.
> >> > +
> >> > +@var{xml_model} is the path of pre-trained classifiers, The C API still
> >> > +does not support the newer cascade format, please use the old format
> >> > +haarcascade_frontalface_alt.xml which type_id is
> >> > opencv-haar-classifier.
> >> > +
> >> > +@var{qoffset}
> >> > +If you want export the detected faces by ROI side data in frame, please
> >> > set
> >> > the
> >> > +parameters, See also the @ref{addroi} filter. The range of qoffset is
> >> > from
> >> > [-1.0, 1.0]
> >> > +
> >> > +By default the filter will report these metadata values if face are
> >> > +detected:
> >> > +@table @option
> >> > +@item lavfi.facedetect.nb_faces
> >> > +Display the detected face number
> >> > +
> >> > +@item lavfi.facedetect.face_id.x, lavfi.facedetect.face_id.y
> >> > +Display x and y of every faces, face_id is the face index which is
> >> > range
> >> > +from [0, nb_faces-1]
> >> > +
> >> > +@item lavfi.facedetect.face_id.w, lavfi.facedetect.face_id.h
> >> > +Display width and height of every faces, face_id is the face index
> >> > +which is range from [0, nb_faces-1]
> >> > +@end table
> >> > +
> >> > @section oscilloscope
> >> >
> >> > 2D Video Oscilloscope.
> >> > diff --git a/libavfilter/vf_libopencv.c b/libavfilter/vf_libopencv.c
> >> > index 8128030b8c..b2d19bb241 100644
> >> > --- a/libavfilter/vf_libopencv.c
> >> > +++ b/libavfilter/vf_libopencv.c
> >> > @@ -1,5 +1,6 @@
> >> > /*
> >> > * Copyright (c) 2010 Stefano Sabatini
> >> > + * Copyright (c) 2020 Limin Wang
> >> > *
> >> > * This file is part of FFmpeg.
> >> > *
> >> > @@ -27,10 +28,16 @@
> >> > #if HAVE_OPENCV2_CORE_CORE_C_H
> >> > #include
> >> > #include
> >> > +#if HAVE_OPENCV2_OBJECTDETECT_OBJECTDETECT_C_H
> >> > +#include
> >> > +#else
> >> > +#include
> >> > +#endif
> >> > #else
> >> > #include
> >> > #include
> >> > #endif
> >> > +
> >> > #include "libavutil/avstring.h"
> >> > #include "libavutil/common.h"
> >> > #include "libavutil/file.h"
> >> > @@ -82,6 +89,7 @@ typedef struct OCVContext {
> >> > int (*init)(AVFilterContext *ctx, const char *args);
> >> > void (*uninit)(AVFilterContext *ctx);
> >> > void (*end_frame_filter)(AVFilterContext *ctx, IplImage *inimg,
> >> > IplImage *outimg);
> >> > +void (*postprocess)(AVFilterContext *ctx, AVFrame *out);
> >> > void *priv;
> >> > } OCVContext;
> >> >
> >> > @@ -326,18 +334,152 @@ static void
> >> > erode_end
Re: [FFmpeg-devel] [PATCH v3 2/5] avfilter/vf_libopencv: add opencv HaarCascade classifier simple face detection filter
Am Mo., 18. Mai 2020 um 13:11 Uhr schrieb Paul B Mahol : > > This opencv module is obsolete for latest opencv. Which is not supported by FFmpeg so I don't think this is a useful argument. > Instead there should be C++ wrapper. If you don't plan to implement the wrapper, it shouldn't be an argument against this patch, no? 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".
Re: [FFmpeg-devel] [PATCH] avutil/video_enc_param: fix warning
Am Do., 21. Mai 2020 um 16:21 Uhr schrieb : > > From: Limin Wang > > warning: comparison is always false due to limited range of data type > [-Wtype-limits] > Also nb_blocks is unsigned int, so nb_blocks * sizeof(AVVideoBlockParams) may > overflow, > so force to size_t > > Signed-off-by: Limin Wang > --- > libavutil/video_enc_params.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c > index c46c0f1..4a4c85f 100644 > --- a/libavutil/video_enc_params.c > +++ b/libavutil/video_enc_params.c > @@ -33,8 +33,8 @@ AVVideoEncParams *av_video_enc_params_alloc(enum > AVVideoEncParamsType type, > size_t size; > > size = sizeof(*par); > -if (nb_blocks > SIZE_MAX / sizeof(AVVideoBlockParams) || > -nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) > +if (nb_blocks > UINT_MAX / sizeof(AVVideoBlockParams) || > +(size_t)nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) I don't think this patch is ok. Shouldn't this be an assert() about sizeof(AVVideoBlockParams) instead? 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".
Re: [FFmpeg-devel] [PATCH] avutil/video_enc_param: fix warning
lance.lmw...@gmail.com (12020-05-21): > From: Limin Wang > > warning: comparison is always false due to limited range of data type > [-Wtype-limits] > Also nb_blocks is unsigned int, so nb_blocks * sizeof(AVVideoBlockParams) may > overflow, > so force to size_t No it may not, the test just before prevents it. > > Signed-off-by: Limin Wang > --- > libavutil/video_enc_params.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c > index c46c0f1..4a4c85f 100644 > --- a/libavutil/video_enc_params.c > +++ b/libavutil/video_enc_params.c > @@ -33,8 +33,8 @@ AVVideoEncParams *av_video_enc_params_alloc(enum > AVVideoEncParamsType type, > size_t size; > > size = sizeof(*par); > -if (nb_blocks > SIZE_MAX / sizeof(AVVideoBlockParams) || > +if (nb_blocks > UINT_MAX / sizeof(AVVideoBlockParams) || These tests are not equivalent. > -nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) > +(size_t)nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) The cast is unnecessary due to C's promotion rules. > return NULL; > size += sizeof(AVVideoBlockParams) * nb_blocks; > 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] avutil/video_enc_param: fix warning
From: Limin Wang warning: comparison is always false due to limited range of data type [-Wtype-limits] Also nb_blocks is unsigned int, so nb_blocks * sizeof(AVVideoBlockParams) may overflow, so force to size_t Signed-off-by: Limin Wang --- libavutil/video_enc_params.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libavutil/video_enc_params.c b/libavutil/video_enc_params.c index c46c0f1..4a4c85f 100644 --- a/libavutil/video_enc_params.c +++ b/libavutil/video_enc_params.c @@ -33,8 +33,8 @@ AVVideoEncParams *av_video_enc_params_alloc(enum AVVideoEncParamsType type, size_t size; size = sizeof(*par); -if (nb_blocks > SIZE_MAX / sizeof(AVVideoBlockParams) || -nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) +if (nb_blocks > UINT_MAX / sizeof(AVVideoBlockParams) || +(size_t)nb_blocks * sizeof(AVVideoBlockParams) > SIZE_MAX - size) return NULL; size += sizeof(AVVideoBlockParams) * nb_blocks; -- 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 v3 2/5] avfilter/vf_libopencv: add opencv HaarCascade classifier simple face detection filter
On 5/18/20, lance.lmw...@gmail.com wrote:
> On Mon, May 18, 2020 at 01:11:12PM +0200, Paul B Mahol wrote:
>> This opencv module is obsolete for latest opencv.
>>
>> Instead there should be C++ wrapper.
>
> Sorry, where is the C++ wrapper? Now Opencv 2.x and 3.x is good
> to use for my current function. By my initial testing, the performance is
> very good, we can easily to get realtime for HD, the drawbacks are:
> 1. The face is not directly opposite and cannot be recognized.
> 2. there are some false detect for video.
>
> If have any old discussion for that, please give the discussion link for
> reference. I'm not keep track of the ML before.
Nope, i'm not doing others people work for free.
>
>
>>
>> On 5/18/20, lance.lmw...@gmail.com wrote:
>> > From: Limin Wang
>> >
>> > Signed-off-by: Limin Wang
>> > ---
>> > change the update_metadata() to postprocess() only, I'll add opencv
>> > drawbox
>> > filter and it need preprocess() to get the meta, so I prefer to change
>> > the
>> > function name for better readablity, in future, it may have other
>> > processing
>> > than metadata only.
>> >
>> > configure | 1 +
>> > doc/filters.texi | 29 +++
>> > libavfilter/vf_libopencv.c | 164 -
>> > 3 files changed, 191 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/configure b/configure
>> > index 34afdaad28..281b67efc4 100755
>> > --- a/configure
>> > +++ b/configure
>> > @@ -2123,6 +2123,7 @@ HEADERS_LIST="
>> > machine_ioctl_meteor_h
>> > malloc_h
>> > opencv2_core_core_c_h
>> > +opencv2_objdetect_objdetect_c_h
>> > OpenGL_gl3_h
>> > poll_h
>> > sys_param_h
>> > diff --git a/doc/filters.texi b/doc/filters.texi
>> > index d9ba0fffa1..f938dd04de 100644
>> > --- a/doc/filters.texi
>> > +++ b/doc/filters.texi
>> > @@ -14177,6 +14177,35 @@ other parameters is 0.
>> > These parameters correspond to the parameters assigned to the
>> > libopencv function @code{cvSmooth}.
>> >
>> > +@subsection facedetect
>> > +Face detection using Haar Feature-based Cascade Classifiers.
>> > +
>> > +The filter takes the following parameters:
>> > +@var{xml_model}|@var{qoffset}.
>> > +
>> > +@var{xml_model} is the path of pre-trained classifiers, The C API still
>> > +does not support the newer cascade format, please use the old format
>> > +haarcascade_frontalface_alt.xml which type_id is
>> > opencv-haar-classifier.
>> > +
>> > +@var{qoffset}
>> > +If you want export the detected faces by ROI side data in frame, please
>> > set
>> > the
>> > +parameters, See also the @ref{addroi} filter. The range of qoffset is
>> > from
>> > [-1.0, 1.0]
>> > +
>> > +By default the filter will report these metadata values if face are
>> > +detected:
>> > +@table @option
>> > +@item lavfi.facedetect.nb_faces
>> > +Display the detected face number
>> > +
>> > +@item lavfi.facedetect.face_id.x, lavfi.facedetect.face_id.y
>> > +Display x and y of every faces, face_id is the face index which is
>> > range
>> > +from [0, nb_faces-1]
>> > +
>> > +@item lavfi.facedetect.face_id.w, lavfi.facedetect.face_id.h
>> > +Display width and height of every faces, face_id is the face index
>> > +which is range from [0, nb_faces-1]
>> > +@end table
>> > +
>> > @section oscilloscope
>> >
>> > 2D Video Oscilloscope.
>> > diff --git a/libavfilter/vf_libopencv.c b/libavfilter/vf_libopencv.c
>> > index 8128030b8c..b2d19bb241 100644
>> > --- a/libavfilter/vf_libopencv.c
>> > +++ b/libavfilter/vf_libopencv.c
>> > @@ -1,5 +1,6 @@
>> > /*
>> > * Copyright (c) 2010 Stefano Sabatini
>> > + * Copyright (c) 2020 Limin Wang
>> > *
>> > * This file is part of FFmpeg.
>> > *
>> > @@ -27,10 +28,16 @@
>> > #if HAVE_OPENCV2_CORE_CORE_C_H
>> > #include
>> > #include
>> > +#if HAVE_OPENCV2_OBJECTDETECT_OBJECTDETECT_C_H
>> > +#include
>> > +#else
>> > +#include
>> > +#endif
>> > #else
>> > #include
>> > #include
>> > #endif
>> > +
>> > #include "libavutil/avstring.h"
>> > #include "libavutil/common.h"
>> > #include "libavutil/file.h"
>> > @@ -82,6 +89,7 @@ typedef struct OCVContext {
>> > int (*init)(AVFilterContext *ctx, const char *args);
>> > void (*uninit)(AVFilterContext *ctx);
>> > void (*end_frame_filter)(AVFilterContext *ctx, IplImage *inimg,
>> > IplImage *outimg);
>> > +void (*postprocess)(AVFilterContext *ctx, AVFrame *out);
>> > void *priv;
>> > } OCVContext;
>> >
>> > @@ -326,18 +334,152 @@ static void
>> > erode_end_frame_filter(AVFilterContext
>> > *ctx, IplImage *inimg, IplIma
>> > cvErode(inimg, outimg, dilate->kernel, dilate->nb_iterations);
>> > }
>> >
>> > +typedef struct FaceDetectContext {
>> > +char *xml_model;
>> > +CvHaarClassifierCascade* cascade;
>> > +CvMemStorage* storage;
>> > +int nb_faces;
>> > +CvSeq *faces;
>> > +int add_roi;
>> > +AVRational qoffset;
>> > +} FaceDetectContext;
>> > +
>> > +static av_cold int facedetect_init(AVFilterContext *ctx, const char
>>
Re: [FFmpeg-devel] [PATCH 1/6] avutil/opt: add AV_OPT_FLAG_CHILD_CONSTS
On Tue, May 19, 2020 at 2:43 AM Marton Balint wrote:
>
>
>
> On Mon, 11 May 2020, Marton Balint wrote:
>
> > This will be used for AVCodecContext->profile. By specifying constants in
> > the
> > encoders we won't have to use the common AVCodecContext options table and
> > different encoders can use the same profile name even with different values.
> >
> > Signed-off-by: Marton Balint
> > ---
> > doc/APIchanges | 3 +++
> > libavutil/opt.c | 3 ++-
> > libavutil/opt.h | 1 +
> > libavutil/version.h | 2 +-
> > 4 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index 75cfdb08b0..235888c174 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -15,6 +15,9 @@ libavutil: 2017-10-21
> >
> > API changes, most recent first:
> >
> > +2020-05-xx - xx - lavu 56.44.101 - opt.h
> > + Add AV_OPT_FLAG_CHILD_CONSTS.
> > +
> > 2020-05-10 - xx - lavu 56.44.100 - hwcontext_vulkan.h
> > Add enabled_inst_extensions, num_enabled_inst_extensions,
> > enabled_dev_extensions
> > and num_enabled_dev_extensions fields to AVVulkanDeviceContext
> > diff --git a/libavutil/opt.c b/libavutil/opt.c
> > index b792dec01c..423313bce2 100644
> > --- a/libavutil/opt.c
> > +++ b/libavutil/opt.c
> > @@ -256,11 +256,12 @@ static int set_string_number(void *obj, void
> > *target_obj, const AVOption *o, con
> > }
> >
> > {
> > -const AVOption *o_named = av_opt_find(target_obj, i ? buf :
> > val, o->unit, 0, 0);
> > int res;
> > int ci = 0;
> > double const_values[64];
> > const char * const_names[64];
> > +int search_flags = (o->flags & AV_OPT_FLAG_CHILD_CONSTS) ?
> > AV_OPT_SEARCH_CHILDREN : 0;
> > +const AVOption *o_named = av_opt_find(target_obj, i ? buf :
> > val, o->unit, 0, search_flags);
> > if (o_named && o_named->type == AV_OPT_TYPE_CONST)
> > d = DEFAULT_NUMVAL(o_named);
> > else {
> > diff --git a/libavutil/opt.h b/libavutil/opt.h
> > index 1969c984dd..e46119572a 100644
> > --- a/libavutil/opt.h
> > +++ b/libavutil/opt.h
> > @@ -291,6 +291,7 @@ typedef struct AVOption {
> > #define AV_OPT_FLAG_RUNTIME_PARAM (1<<15) ///< a generic parameter which
> > can be set by the user at runtime
> > #define AV_OPT_FLAG_FILTERING_PARAM (1<<16) ///< a generic parameter which
> > can be set by the user for filtering
> > #define AV_OPT_FLAG_DEPRECATED (1<<17) ///< set if option is
> > deprecated, users should refer to AVOption.help text for more information
> > +#define AV_OPT_FLAG_CHILD_CONSTS(1<<18) ///< set if option constants
> > can also reside in child objects
> > //FIXME think about enc-audio, ... style flags
> >
> > /**
> > diff --git a/libavutil/version.h b/libavutil/version.h
> > index 48d8a38c42..c4946c1c7e 100644
> > --- a/libavutil/version.h
> > +++ b/libavutil/version.h
> > @@ -80,7 +80,7 @@
> >
> > #define LIBAVUTIL_VERSION_MAJOR 56
> > #define LIBAVUTIL_VERSION_MINOR 44
> > -#define LIBAVUTIL_VERSION_MICRO 100
> > +#define LIBAVUTIL_VERSION_MICRO 101
> >
> > #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> >LIBAVUTIL_VERSION_MINOR, \
>
>
> Ping for the series, is there anybody against the approach I took to move
> profile name constants to encoders?
I'm OK for this series
___
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/6] avutil/opt: add AV_OPT_FLAG_CHILD_CONSTS
Quoting Marton Balint (2020-05-18 20:43:45) > Ping for the series, is there anybody against the approach I took to move > profile name constants to encoders? I like it. Wondered if sharing options like that might cause problems but couldn't think of anything. -- Anton Khirnov ___ 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/3] avcodec: deprecate Lossless and Intra Only encoder capabilites
Quoting James Almer (2020-05-18 18:33:26) > Both are codec properties and not encoder capabilities. The relevant > AVCodecDescriptor.props flags exist for this purpose. > > Signed-off-by: James Almer > --- Don't know if it makes sense to bump version just because something was deprecated. Other than that looks ok. -- Anton Khirnov ___ 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] [RFC] encoder profile validation
Quoting Marton Balint (2020-05-20 20:26:53)
>
>
> You asked that why the AVProfiles list of the encoders should be different
> to the AVProfile lists stored in libavcodec/profiles.c. Or at least that's
> how I understood your question...
>
> Anyway, I'd still argue against the usage of AVCodec->profiles
> and AVProfiles in general. Because in an encoder we'd have to list the
> supported profiles as AVOptions in order to be able to specify the profile
> names:
>
> (See my previous patchset [1] for the macro)
>
> static const AVOption options[] = {
> FF_AVCTX_PROFILE_OPTION("dnxhd", NULL, VIDEO, FF_PROFILE_DNXHD)
> FF_AVCTX_PROFILE_OPTION("dnxhr_444", NULL, VIDEO, FF_PROFILE_DNXHR_444)
> FF_AVCTX_PROFILE_OPTION("dnxhr_hqx", NULL, VIDEO, FF_PROFILE_DNXHR_HQX)
> FF_AVCTX_PROFILE_OPTION("dnxhr_hq", NULL, VIDEO, FF_PROFILE_DNXHR_HQ)
> FF_AVCTX_PROFILE_OPTION("dnxhr_sq", NULL, VIDEO, FF_PROFILE_DNXHR_SQ)
> FF_AVCTX_PROFILE_OPTION("dnxhr_lb", NULL, VIDEO, FF_PROFILE_DNXHR_LB)
> { NULL }
> }
>
> And we'd also have to list the supported profiles as AVProfiles to specify
> the valid profiles.
>
> const AVProfile profiles[] = {
>{ FF_PROFILE_DNXHD, "DNXHD"},
>{ FF_PROFILE_DNXHR_LB, "DNXHR LB"},
>{ FF_PROFILE_DNXHR_SQ, "DNXHR SQ"},
>{ FF_PROFILE_DNXHR_HQ, "DNXHR HQ" },
>{ FF_PROFILE_DNXHR_HQX, "DNXHR HQX"},
>{ FF_PROFILE_DNXHR_444, "DNXHR 444"},
>{ FF_PROFILE_UNKNOWN },
> };
>
> So we'd have the list of supported profiles in an encoder two times which
> seems ugly and can become inconsistent. Also it seems cleaner to me to to do
> the
> profile validation via AVOptions, rather than hardcoding checking the
> profiles list in avcodec_open2.
I suppose that makes sense, but then AVCodec.profiles should be
deprecated, otherwise users will be confused about what to use.
--
Anton Khirnov
___
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/nvenc: set average and max bitrate values to zero when in CQ rate control mode
On 18.05.2020 09:54, Roman Arzumanyan wrote: Hello, The patch fixes bug with CQ RC mode: max and average bitrate values were not set to zero. How to reproduce: #CQ 30 file ffmpeg \ -i big_buck_bunny_1080p_h264.mov -c:v h264_nvenc -preset medium \ -cq 30 -y output_cq30.mp4 #CQ 10 file ffmpeg \ -i big_buck_bunny_1080p_h264.mov -c:v h264_nvenc -preset medium \ -cq 10 -y output_cq10.mp4 Before the patch, both output_cq30.mp4 and output_cq10.mp4 were of same size. After the patch, output_cq10.mp4 file has higher quality and bigger size. -- BR, Roman Arzumanyan Looks generally ok, but I want to give it a few tests and haven't found the time for that yet. ___ 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] fate: add tests for h264 and vp9 video enc parameters export
---
libavformat/Makefile | 1 +
tests/Makefile| 1 +
tests/fate-run.sh | 7 +
tests/fate/h264.mak | 6 +-
tests/fate/vpx.mak| 5 +
tests/ref/fate/h264-encparams | 404 +++
tests/ref/fate/vp9-encparams | 937 ++
tools/venc_data_dump.c| 195 +++
8 files changed, 1555 insertions(+), 1 deletion(-)
create mode 100644 tests/ref/fate/h264-encparams
create mode 100644 tests/ref/fate/vp9-encparams
create mode 100644 tools/venc_data_dump.c
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 5fa24cef16..67778ff798 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -670,3 +670,4 @@ TOOLS = aviocat
\
probetest \
seek_print \
sidxindex \
+venc_data_dump
diff --git a/tests/Makefile b/tests/Makefile
index ab3c235f25..7844901e53 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -221,6 +221,7 @@ $(FATE_FFMPEG) $(FATE_FFMPEG_FFPROBE)
$(FATE_SAMPLES_AVCONV) $(FATE_SAMPLES_FFMP
$(FATE_FFPROBE) $(FATE_FFMPEG_FFPROBE) $(FATE_SAMPLES_FFPROBE)
$(FATE_SAMPLES_FFMPEG_FFPROBE): ffprobe$(PROGSSUF)$(EXESUF)
$(FATE_SAMPLES_FASTSTART): tools/qt-faststart$(EXESUF)
+$(FATE_SAMPLES_DUMP_DATA): tools/venc_data_dump$(EXESUF)
ifdef SAMPLES
FATE += $(FATE_EXTERN)
diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 7c6d753261..002944b010 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -487,6 +487,13 @@ concat(){
fi
}
+venc_data(){
+file=$1
+stream=$2
+frames=$3
+run tools/venc_data_dump${EXECSUF} ${file} ${stream} ${frames} ${threads}
${thread_type}
+}
+
null(){
:
}
diff --git a/tests/fate/h264.mak b/tests/fate/h264.mak
index f14b46c6e0..13a596e52b 100644
--- a/tests/fate/h264.mak
+++ b/tests/fate/h264.mak
@@ -196,7 +196,8 @@ FATE_H264 := $(FATE_H264:%=fate-h264-conformance-%)
\
fate-h264-3386\
fate-h264-missing-frame \
fate-h264-ref-pic-mod-overflow\
- fate-h264-timecode
+ fate-h264-timecode\
+ fate-h264-encparams
FATE_H264-$(call DEMDEC, H264, H264) += $(FATE_H264)
FATE_H264-$(call DEMDEC, MOV, H264) += fate-h264-crop-to-container
@@ -446,3 +447,6 @@ fate-h264-timecode: CMD =
framecrc -i $(TARGET_SAM
fate-h264-reinit-%: CMD = framecrc -i
$(TARGET_SAMPLES)/h264/$(@:fate-h264-%=%).h264 -vf
format=yuv444p10le,scale=w=352:h=288
fate-h264-dts_5frames:CMD = probeframes
$(TARGET_SAMPLES)/h264/dts_5frames.mkv
+
+fate-h264-encparams: CMD = venc_data
$(TARGET_SAMPLES)/h264-conformance/FRext/FRExt_MMCO4_Sony_B.264 0 1
+FATE_SAMPLES_DUMP_DATA += fate-h264-encparams
diff --git a/tests/fate/vpx.mak b/tests/fate/vpx.mak
index c65959f133..88ccc70a5a 100644
--- a/tests/fate/vpx.mak
+++ b/tests/fate/vpx.mak
@@ -163,5 +163,10 @@ FATE_VP9-$(CONFIG_IVF_DEMUXER) += fate-vp9-05-resize
fate-vp9-05-resize: CMD = framemd5 -i
$(TARGET_SAMPLES)/vp9-test-vectors/vp90-2-05-resize.ivf -s 352x288 -sws_flags
bitexact+bilinear
fate-vp9-05-resize: REF = $(SRC_PATH)/tests/ref/fate/vp9-05-resize
+fate-vp9-encparams: CMD = venc_data
$(TARGET_SAMPLES)/vp9-test-vectors/vp90-2-segmentation-aq-akiyo.webm 0 5
+FATE_SAMPLES_DUMP_DATA += fate-vp9-encparams
+
+FATE_VP9-$(CONFIG_MATROSKA_DEMUXER) += fate-vp9-encparams
+
FATE_SAMPLES_AVCONV-$(CONFIG_VP9_DECODER) += $(FATE_VP9-yes)
fate-vp9: $(FATE_VP9-yes)
diff --git a/tests/ref/fate/h264-encparams b/tests/ref/fate/h264-encparams
new file mode 100644
index 00..8dd5e86b74
--- /dev/null
+++ b/tests/ref/fate/h264-encparams
@@ -0,0 +1,404 @@
+frame 0
+AVVideoEncParams 1
+qp 28
+delta_qp[1][0] 3
+delta_qp[1][1] 3
+delta_qp[2][0] -2
+delta_qp[2][1] -2
+nb_blocks 396
+block 0 0:0 16x16 0
+block 1 16:0 16x16 0
+block 2 32:0 16x16 0
+block 3 48:0 16x16 0
+block 4 64:0 16x16 0
+block 5 80:0 16x16 0
+block 6 96:0 16x16 0
+block 7 112:0 16x16 0
+block 8 128:0 16x16 0
+block 9 144:0 16x16 0
+block 10 160:0 16x16 0
+block 11 176:0 16x16 0
+block 12 192:0 16x16 0
+block 13 208:0 16x16 0
+block 14 224:0 16x16 0
+block 15 240:0 16x16 0
+block 16 256:0 16x16 0
+block 17 272:0 16x16 0
+block 18 288:0 16x16 0
+block 19 304:0 16x16 0
+block 20 320:0 16x16 0
+block 21 336:0 16x16 0
+block 22 0:16 16x16 0
+block 23 16:16 16x16 0
+block 24 32:16 16x16 0
+block 25 48:16 16x16 0
+block 26 64:16 16x16 0
+block 27 80:16 16x16 0
+block 28 96:16 16x16 0
+block 29 112:16 16x16 0
+block 30 128:16 16x16 0
+block 31 144:16 16x16 0
+block 32 160:16 1
Re: [FFmpeg-devel] [PATCH] avcodec/tiff: Check for Tiled and Stripped TIFFs
On Thu, May 21, 2020 at 12:13:15PM +0200, Anton Khirnov wrote: > Quoting Michael Niedermayer (2020-05-21 08:59:27) > > > > I dont see IS_IRAP_NAL() and IS_IDR_NAL() to check every field in the > > headers > > to be consistent with IDR / IRAP NALs, which is what the tiff code in this > > thread does kind off .. > > > > to match hevc, i could add something like > > #define HAVE_TILES(s) s->has_tiles > > #define HAVE_STRIPS(s) s->strips > > > > but iam not sure what that would simplify > > The validity checks are needed once and if thats put in a macro then maybe > > that > > could be called HAVE_RANDOM_TILE_BITS() and HAVE_RANDOM_STRIP_BITS() could > > be > > maybe a possibility > > then again if thats what is suggested then i would suggest to rather add 2 > > local variable with these names instead of a macro which would seem more > > readable > > > > whats your opinion ? > > I see little difference between a macro and a local variable in this > case, either one is fine with me. And both are significantly more > readable than a giant condition that the reader has to reverse engineer > into (have_tiles && have_strips) ok, will apply it with local variables thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB In fact, the RIAA has been known to suggest that students drop out of college or go to community college in order to be able to afford settlements. -- The RIAA 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] movenc: Use first H264/HEVC frame as extradata, if it is missing
On Thu, 21 May 2020, Michael Niedermayer wrote:
On Wed, May 20, 2020 at 09:10:21AM +0300, Martin Storsjö wrote:
Sticking a full frame in the extradata works, as the code for writing
the avcC/hvcC extracts the relevant parameter set NAL units - provided
that they actually exist in the frame.
Some encoders don't provide split out extradata directly on init (or
at all). In particular, the MediaFoundation encoder wrapper doesn't
always (depending on the actual encoder device) - this is the case for
Qualcomm's HEVC encoder on SD835, and also on some QSV H264 encoders).
This only works for cases where the moov hasn't already been written
(e.g. when not writing fragmented mp4 with empty_moov, unless using
the delay_moov option).
Signed-off-by: Martin Storsjö
---
libavformat/movenc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 27d7621e27..6a85440a3f 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5584,7 +5584,9 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
if ((par->codec_id == AV_CODEC_ID_DNXHD ||
par->codec_id == AV_CODEC_ID_TRUEHD ||
- par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len) {
+ par->codec_id == AV_CODEC_ID_AC3 ||
+ par->codec_id == AV_CODEC_ID_H264 ||
+ par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
/* copy frame to create needed atoms */
trk->vos_len = size;
trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
This changes avcintra output
example testcase:
./ffmpeg -i ~/videos/mm-short.mpg -avcintra-class 100 -tune psnr -flags
+ildct-global_header -t 0.5 -pix_fmt yuv422p10 -vf scale=1920:1080 -an file.mov
I believe this should be fixed by "[PATCH v2] movenc: Fix conversion of
the first frame for extradata-less H264/HEVC".
// 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] movenc: Fix conversion of the first frame for extradata-less H264/HEVC
Move the copying of the frame to vos_data further up in the function,
so that when writing the actual frame data for the first frame, it's
clear that the stream really is in annex b format, for the cases where
we create extradata from the first frame.
Alternatively - we could invert the checks for bitstream format. If
extradata is missing, we can't pretend that the bitstream is in
mp4 form, because we can't even know the NAL unit length prefix size
in that case.
Also avoid creating extradata for AVC intra. If the track tag is
an AVC intra tag, don't copy the frame into vos_data - this matches
other existing cases of how vos_data and TAG_IS_AVCI interact in
other places.
---
libavformat/movenc.c | 37 +++--
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 4560064add..2e164a106f 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5504,6 +5504,25 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket
*pkt)
memset(trk->vos_data + trk->vos_len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
}
+if ((par->codec_id == AV_CODEC_ID_DNXHD ||
+ par->codec_id == AV_CODEC_ID_H264 ||
+ par->codec_id == AV_CODEC_ID_HEVC ||
+ par->codec_id == AV_CODEC_ID_TRUEHD ||
+ par->codec_id == AV_CODEC_ID_AC3 ||
+ par->codec_id == AV_CODEC_ID_H264 ||
+ par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len &&
+ !TAG_IS_AVCI(trk->tag)) {
+/* copy frame to create needed atoms */
+trk->vos_len = size;
+trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
+if (!trk->vos_data) {
+ret = AVERROR(ENOMEM);
+goto err;
+}
+memcpy(trk->vos_data, pkt->data, size);
+memset(trk->vos_data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
+}
+
if (par->codec_id == AV_CODEC_ID_AAC && pkt->size > 2 &&
(AV_RB16(pkt->data) & 0xfff0) == 0xfff0) {
if (!s->streams[pkt->stream_index]->nb_frames) {
@@ -5582,24 +5601,6 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket
*pkt)
}
}
-if ((par->codec_id == AV_CODEC_ID_DNXHD ||
- par->codec_id == AV_CODEC_ID_H264 ||
- par->codec_id == AV_CODEC_ID_HEVC ||
- par->codec_id == AV_CODEC_ID_TRUEHD ||
- par->codec_id == AV_CODEC_ID_AC3 ||
- par->codec_id == AV_CODEC_ID_H264 ||
- par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
-/* copy frame to create needed atoms */
-trk->vos_len = size;
-trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
-if (!trk->vos_data) {
-ret = AVERROR(ENOMEM);
-goto err;
-}
-memcpy(trk->vos_data, pkt->data, size);
-memset(trk->vos_data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
-}
-
if (trk->entry >= trk->cluster_capacity) {
unsigned new_capacity = trk->entry + MOV_INDEX_CLUSTER_SIZE;
if (av_reallocp_array(&trk->cluster, new_capacity,
--
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 v2] movenc: Use first H264/HEVC frame as extradata, if it is missing
On Thu, 21 May 2020, Jan Ekström wrote: On Thu, May 21, 2020 at 2:00 PM Martin Storsjö wrote: Oh, indeed, you're right. However wrong that is from the mov/mp4 point of view, it may very well be that AVC intra is supposed to be written like this in mov/mp4 files - but we'd need a word from somebody who actually knows that case. The best way to figure out if a user wants to do AVC Intra muxing would be to either a) require specific codec_tag/muxing mode be set by user or b) attempt to somehow properly check the actual bit stream within `mov_get_h264_codec_tag` to make the selection proper. Right now the system seems to base that if you are doing H.264, and have no codec_tag set already, then `mov_get_h264_codec_tag` attempts to set an AVC Intra codec_tag (which hits TAG_IS_AVCI), which then attempts to mux things in that proprietary way. Thanks for the insight into this case, that it really is intended to mux it this way. I hadn't known what TAG_IS_AVCI is really, but with that it should be easy to fix this case. // 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 v2] movenc: Use first H264/HEVC frame as extradata, if it is missing
On Thu, May 21, 2020 at 2:00 PM Martin Storsjö wrote: > > Oh, indeed, you're right. > > However wrong that is from the mov/mp4 point of view, it may very well be > that AVC intra is supposed to be written like this in mov/mp4 files - but > we'd need a word from somebody who actually knows that case. > The best way to figure out if a user wants to do AVC Intra muxing would be to either a) require specific codec_tag/muxing mode be set by user or b) attempt to somehow properly check the actual bit stream within `mov_get_h264_codec_tag` to make the selection proper. Right now the system seems to base that if you are doing H.264, and have no codec_tag set already, then `mov_get_h264_codec_tag` attempts to set an AVC Intra codec_tag (which hits TAG_IS_AVCI), which then attempts to mux things in that proprietary way. Currently the check only checks for 10bit 4:2:0/4:2:2 or specific resolutions (which include 1080p and such). And I'm not sure if even the fallback there is a standard codec_tag, since it is not one of those I remember from 14496-15 (although my memory might just be bad). By default, and if the output got written with a standard 14496-15 etc codec_tag, then AVC intra muxing mode should not utilized. And just a note for the record, the logic in `mov_get_h264_codec_tag` seems to already be somewhat imperfect, as I've seen users attempting to encode 10bit H.264 normally into MOV, and one had to force a codec_tag to be avc1 for a non-broken file to be generated (as in, even FFmpeg itself couldn't read the file after muxing with the default selected codec_tag). Just my two cents, Jan ___ 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] movenc: Fix conversion of the first frame for extradata-less H264/HEVC
Move the copying of the frame to vos_data further up in the function,
so that when writing the actual frame data for the first frame, it's
clear that the stream really is in annex b format, for the cases where
we create extradata from the first frame.
Alternatively - we could invert the checks for bitstream format. If
extradata is missing, we can't pretend that the bitstream is in
mp4 form, because we can't even know the NAL unit length prefix size
in that case.
---
libavformat/movenc.c | 36 ++--
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 4560064add..b61ae7aad9 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5504,6 +5504,24 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket
*pkt)
memset(trk->vos_data + trk->vos_len, 0, AV_INPUT_BUFFER_PADDING_SIZE);
}
+if ((par->codec_id == AV_CODEC_ID_DNXHD ||
+ par->codec_id == AV_CODEC_ID_H264 ||
+ par->codec_id == AV_CODEC_ID_HEVC ||
+ par->codec_id == AV_CODEC_ID_TRUEHD ||
+ par->codec_id == AV_CODEC_ID_AC3 ||
+ par->codec_id == AV_CODEC_ID_H264 ||
+ par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
+/* copy frame to create needed atoms */
+trk->vos_len = size;
+trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
+if (!trk->vos_data) {
+ret = AVERROR(ENOMEM);
+goto err;
+}
+memcpy(trk->vos_data, pkt->data, size);
+memset(trk->vos_data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
+}
+
if (par->codec_id == AV_CODEC_ID_AAC && pkt->size > 2 &&
(AV_RB16(pkt->data) & 0xfff0) == 0xfff0) {
if (!s->streams[pkt->stream_index]->nb_frames) {
@@ -5582,24 +5600,6 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket
*pkt)
}
}
-if ((par->codec_id == AV_CODEC_ID_DNXHD ||
- par->codec_id == AV_CODEC_ID_H264 ||
- par->codec_id == AV_CODEC_ID_HEVC ||
- par->codec_id == AV_CODEC_ID_TRUEHD ||
- par->codec_id == AV_CODEC_ID_AC3 ||
- par->codec_id == AV_CODEC_ID_H264 ||
- par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
-/* copy frame to create needed atoms */
-trk->vos_len = size;
-trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
-if (!trk->vos_data) {
-ret = AVERROR(ENOMEM);
-goto err;
-}
-memcpy(trk->vos_data, pkt->data, size);
-memset(trk->vos_data + size, 0, AV_INPUT_BUFFER_PADDING_SIZE);
-}
-
if (trk->entry >= trk->cluster_capacity) {
unsigned new_capacity = trk->entry + MOV_INDEX_CLUSTER_SIZE;
if (av_reallocp_array(&trk->cluster, new_capacity,
--
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 v2] movenc: Use first H264/HEVC frame as extradata, if it is missing
On Thu, 21 May 2020, Andreas Rheinhardt wrote:
Martin Storsjö:
On Thu, 21 May 2020, Andreas Rheinhardt wrote:
Martin Storsjö:
On Thu, 21 May 2020, Michael Niedermayer wrote:
On Wed, May 20, 2020 at 09:10:21AM +0300, Martin Storsjö wrote:
Sticking a full frame in the extradata works, as the code for writing
the avcC/hvcC extracts the relevant parameter set NAL units - provided
that they actually exist in the frame.
Some encoders don't provide split out extradata directly on init (or
at all). In particular, the MediaFoundation encoder wrapper doesn't
always (depending on the actual encoder device) - this is the case for
Qualcomm's HEVC encoder on SD835, and also on some QSV H264 encoders).
This only works for cases where the moov hasn't already been written
(e.g. when not writing fragmented mp4 with empty_moov, unless using
the delay_moov option).
Signed-off-by: Martin Storsjö
---
libavformat/movenc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 27d7621e27..6a85440a3f 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5584,7 +5584,9 @@ int ff_mov_write_packet(AVFormatContext *s,
AVPacket *pkt)
if ((par->codec_id == AV_CODEC_ID_DNXHD ||
par->codec_id == AV_CODEC_ID_TRUEHD ||
- par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len) {
+ par->codec_id == AV_CODEC_ID_AC3 ||
+ par->codec_id == AV_CODEC_ID_H264 ||
+ par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
/* copy frame to create needed atoms */
trk->vos_len = size;
trk->vos_data = av_malloc(size +
AV_INPUT_BUFFER_PADDING_SIZE);
This changes avcintra output
example testcase:
./ffmpeg -i ~/videos/mm-short.mpg -avcintra-class 100 -tune psnr
-flags +ildct-global_header -t 0.5 -pix_fmt yuv422p10 -vf
scale=1920:1080 -an file.mov
Right, if explicitly disabling global headers on the encoder, this will
end up putting them back.
Any ideas on what the best path forward would be?
// Martin
If I am not mistaken, then the sample created by the above command line
is annex b in mp4 which is against the spec. So changing the output is
nothing to worry about.
No, the mov muxer does rewrite the bitstream from annex b to mp4 form,
regardless of whether there was any broken out parameter sets in the
extradata.
Sure about that? The check used is
if (par->codec_id == AV_CODEC_ID_H264 && trk->vos_len > 0 &&
*(uint8_t *)trk->vos_data != 1 && !TAG_IS_AVCI(trk->tag)) {
If trk->vos_len is zero (which is a requirement for your patch to have
any effect), no reformatting will be performed.
Oh, indeed, you're right.
However wrong that is from the mov/mp4 point of view, it may very well be
that AVC intra is supposed to be written like this in mov/mp4 files - but
we'd need a word from somebody who actually knows that case.
Hmm, and unfortunately, this check is further up in the function, than
when creating the vos_data/vos_len from the frame, so the first frame
doesn't end up converted correctly in my case... Will send a new patch to
fix that.
// 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 v2] movenc: Use first H264/HEVC frame as extradata, if it is missing
Martin Storsjö:
> On Thu, 21 May 2020, Andreas Rheinhardt wrote:
>
>> Martin Storsjö:
>>> On Thu, 21 May 2020, Michael Niedermayer wrote:
>>>
On Wed, May 20, 2020 at 09:10:21AM +0300, Martin Storsjö wrote:
> Sticking a full frame in the extradata works, as the code for writing
> the avcC/hvcC extracts the relevant parameter set NAL units - provided
> that they actually exist in the frame.
>
> Some encoders don't provide split out extradata directly on init (or
> at all). In particular, the MediaFoundation encoder wrapper doesn't
> always (depending on the actual encoder device) - this is the case for
> Qualcomm's HEVC encoder on SD835, and also on some QSV H264 encoders).
>
> This only works for cases where the moov hasn't already been written
> (e.g. when not writing fragmented mp4 with empty_moov, unless using
> the delay_moov option).
>
> Signed-off-by: Martin Storsjö
> ---
> libavformat/movenc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 27d7621e27..6a85440a3f 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -5584,7 +5584,9 @@ int ff_mov_write_packet(AVFormatContext *s,
> AVPacket *pkt)
>
> if ((par->codec_id == AV_CODEC_ID_DNXHD ||
> par->codec_id == AV_CODEC_ID_TRUEHD ||
> - par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len) {
> + par->codec_id == AV_CODEC_ID_AC3 ||
> + par->codec_id == AV_CODEC_ID_H264 ||
> + par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
> /* copy frame to create needed atoms */
> trk->vos_len = size;
> trk->vos_data = av_malloc(size +
> AV_INPUT_BUFFER_PADDING_SIZE);
This changes avcintra output
example testcase:
./ffmpeg -i ~/videos/mm-short.mpg -avcintra-class 100 -tune psnr
-flags +ildct-global_header -t 0.5 -pix_fmt yuv422p10 -vf
scale=1920:1080 -an file.mov
>>>
>>> Right, if explicitly disabling global headers on the encoder, this will
>>> end up putting them back.
>>>
>>> Any ideas on what the best path forward would be?
>>>
>>> // Martin
>>
>> If I am not mistaken, then the sample created by the above command line
>> is annex b in mp4 which is against the spec. So changing the output is
>> nothing to worry about.
>
> No, the mov muxer does rewrite the bitstream from annex b to mp4 form,
> regardless of whether there was any broken out parameter sets in the
> extradata.
>
Sure about that? The check used is
if (par->codec_id == AV_CODEC_ID_H264 && trk->vos_len > 0 &&
*(uint8_t *)trk->vos_data != 1 && !TAG_IS_AVCI(trk->tag)) {
If trk->vos_len is zero (which is a requirement for your patch to have
any effect), no reformatting will be performed.
- 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 v2] movenc: Use first H264/HEVC frame as extradata, if it is missing
On Thu, 21 May 2020, Andreas Rheinhardt wrote:
Martin Storsjö:
On Thu, 21 May 2020, Michael Niedermayer wrote:
On Wed, May 20, 2020 at 09:10:21AM +0300, Martin Storsjö wrote:
Sticking a full frame in the extradata works, as the code for writing
the avcC/hvcC extracts the relevant parameter set NAL units - provided
that they actually exist in the frame.
Some encoders don't provide split out extradata directly on init (or
at all). In particular, the MediaFoundation encoder wrapper doesn't
always (depending on the actual encoder device) - this is the case for
Qualcomm's HEVC encoder on SD835, and also on some QSV H264 encoders).
This only works for cases where the moov hasn't already been written
(e.g. when not writing fragmented mp4 with empty_moov, unless using
the delay_moov option).
Signed-off-by: Martin Storsjö
---
libavformat/movenc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 27d7621e27..6a85440a3f 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5584,7 +5584,9 @@ int ff_mov_write_packet(AVFormatContext *s,
AVPacket *pkt)
if ((par->codec_id == AV_CODEC_ID_DNXHD ||
par->codec_id == AV_CODEC_ID_TRUEHD ||
- par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len) {
+ par->codec_id == AV_CODEC_ID_AC3 ||
+ par->codec_id == AV_CODEC_ID_H264 ||
+ par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
/* copy frame to create needed atoms */
trk->vos_len = size;
trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
This changes avcintra output
example testcase:
./ffmpeg -i ~/videos/mm-short.mpg -avcintra-class 100 -tune psnr
-flags +ildct-global_header -t 0.5 -pix_fmt yuv422p10 -vf
scale=1920:1080 -an file.mov
Right, if explicitly disabling global headers on the encoder, this will
end up putting them back.
Any ideas on what the best path forward would be?
// Martin
If I am not mistaken, then the sample created by the above command line
is annex b in mp4 which is against the spec. So changing the output is
nothing to worry about.
No, the mov muxer does rewrite the bitstream from annex b to mp4 form,
regardless of whether there was any broken out parameter sets in the
extradata.
I presume the intent is that with libx264, normally you'd have parameter
sets prepended on each IDR frame. If the global headers flag is set, the
parameter sets are placed in extradata and not prepended on each frame.
And I guess it's needed for the AVC intra spec to have the parameter sets
included in each frame, instead of tucked away in the avc1 global header.
I guess the main question is, does it matter and/or does it break
anything, if we now include parameter sets in avc1 like a normal mov/mp4,
for AVC intra, while the parameter sets still are available within the
frames as well.
// 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] avformat/aadec: Check toc_size to contain the minimum to demuxer uses
On Tue, Apr 07, 2020 at 01:03:34PM +0200, Michael Niedermayer wrote: > Fixes: out of array access > Fixes: stack-buffer-overflow-READ-0x0831fff1 > > Found-by: GalyCannon > Signed-off-by: Michael Niedermayer > --- > libavformat/aadec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Many that live deserve death. And some that die deserve life. Can you give it to them? Then do not be too eager to deal out death in judgement. For even the very wise cannot see all ends. -- Gandalf 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 7/7] avcodec/bitpacked: Add codec_tags list
On Fri, Feb 07, 2020 at 02:48:31PM +0100, Michael Niedermayer wrote: > This should improve coverage > > Signed-off-by: Michael Niedermayer > --- > libavcodec/bitpacked.c | 4 > 1 file changed, 4 insertions(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Those who are too smart to engage in politics are punished by being governed by those who are dumber. -- Plato 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/4] avformat/mpegenc: Fix integer overflow with AV_NOPTS_VALUE
On Sun, Feb 16, 2020 at 08:43:01PM +0100, Michael Niedermayer wrote: > Fixes: signed integer overflow: -9223372036854775808 - 45000 cannot be > represented in type 'long' > Fixes: ticket8187 > > Found-by: Suhwan > Signed-off-by: Michael Niedermayer > --- > libavformat/mpegenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Republics decline into democracies and democracies degenerate into despotisms. -- 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 1/4] avformat/swfenc: Fix integer overflow in frame rate handling
On Sun, Feb 16, 2020 at 08:43:00PM +0100, Michael Niedermayer wrote: > Fixes: signed integer overflow: 3299 * 256 cannot be represented in type > 'int' > Fixes: ticket8184 > > Found-by: Suhwan > Signed-off-by: Michael Niedermayer > --- > libavformat/swfenc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Dictatorship naturally arises out of democracy, and the most aggravated form of tyranny and slavery out of the most extreme liberty. -- Plato 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] avcodec/wmalosslessdec: Fix integer overflows in revert_inter_ch_decorr()
On Sun, Apr 26, 2020 at 11:12:17PM +0200, Michael Niedermayer wrote: > Fixes: signed integer overflow: -717241856 + -1434459904 cannot be > represented in type 'int' > Fixes: > 21405/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_WMALOSSLESS_fuzzer-5677143666458624 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/wmalosslessdec.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Democracy is the form of government in which you can choose your dictator 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] avcodec/cbs_jpeg: Fix infinite loop in cbs_jpeg_split_fragment()
On Fri, Mar 20, 2020 at 01:03:35AM +0100, Michael Niedermayer wrote: > Fixes: Timeout > Fixes: > 21104/clusterfuzz-testcase-minimized-ffmpeg_BSF_TRACE_HEADERS_fuzzer-5129580475318272 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/cbs_jpeg.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes 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] avcodec/xvididct: Fix integer overflow in idct_row()
On Sat, May 02, 2020 at 11:08:07PM +0200, Michael Niedermayer wrote: > Fixes: signed integer overflow: -1238335488 + -1003634688 cannot be > represented in type 'int' > Fixes: > 21649/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_MJPEG_fuzzer-5112005765890048 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/xvididct.c | 36 ++-- > 1 file changed, 18 insertions(+), 18 deletions(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Modern terrorism, a quick summary: Need oil, start war with country that has oil, kill hundread thousand in war. Let country fall into chaos, be surprised about raise of fundamantalists. Drop more bombs, kill more people, be surprised about them taking revenge and drop even more bombs and strip your own citizens of their rights and freedoms. to be continued 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/dsddec: Check channels
On Mon, May 04, 2020 at 10:10:00PM +0200, Michael Niedermayer wrote: > Fixes: division by zero > Fixes: > 21677/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_DSD_MSBF_fuzzer-5712547983654912 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/dsddec.c | 3 +++ > 1 file changed, 3 insertions(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Modern terrorism, a quick summary: Need oil, start war with country that has oil, kill hundread thousand in war. Let country fall into chaos, be surprised about raise of fundamantalists. Drop more bombs, kill more people, be surprised about them taking revenge and drop even more bombs and strip your own citizens of their rights and freedoms. to be continued 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/pnmdec: Use unsigned for maxval rescaling
On Sat, May 16, 2020 at 06:37:32PM +0200, Michael Niedermayer wrote: > Fixes: signed integer overflow: 65535 * 55335 cannot be represented in type > 'int' > Fixes: > 21955/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_PGMYUV_fuzzer-5669206981083136 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/pnmdec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB If a bugfix only changes things apparently unrelated to the bug with no further explanation, that is a good sign that the bugfix is wrong. 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/ivi: Clear got_p_frame before decoding a new frame using it
On Wed, May 13, 2020 at 01:06:26AM +0200, Michael Niedermayer wrote: > Fixes: assertion failure > Fixes: > 21666/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_INDEO4_fuzzer-5706468994318336 > > Found-by: continuous fuzzing process > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg > Signed-off-by: Michael Niedermayer > --- > libavcodec/ivi.c | 2 ++ > 1 file changed, 2 insertions(+) will apply [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Frequently ignored answer#1 FFmpeg bugs should be sent to our bugtracker. User questions about the command line tools should be sent to the ffmpeg-user ML. And questions about how to use libav* should be sent to the libav-user ML. 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/tiff: Check for Tiled and Stripped TIFFs
Quoting Michael Niedermayer (2020-05-21 08:59:27) > > I dont see IS_IRAP_NAL() and IS_IDR_NAL() to check every field in the headers > to be consistent with IDR / IRAP NALs, which is what the tiff code in this > thread does kind off .. > > to match hevc, i could add something like > #define HAVE_TILES(s) s->has_tiles > #define HAVE_STRIPS(s) s->strips > > but iam not sure what that would simplify > The validity checks are needed once and if thats put in a macro then maybe > that > could be called HAVE_RANDOM_TILE_BITS() and HAVE_RANDOM_STRIP_BITS() could be > maybe a possibility > then again if thats what is suggested then i would suggest to rather add 2 > local variable with these names instead of a macro which would seem more > readable > > whats your opinion ? I see little difference between a macro and a local variable in this case, either one is fine with me. And both are significantly more readable than a giant condition that the reader has to reverse engineer into (have_tiles && have_strips) -- Anton Khirnov ___ 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] movenc: Use first H264/HEVC frame as extradata, if it is missing
Martin Storsjö:
> On Thu, 21 May 2020, Michael Niedermayer wrote:
>
>> On Wed, May 20, 2020 at 09:10:21AM +0300, Martin Storsjö wrote:
>>> Sticking a full frame in the extradata works, as the code for writing
>>> the avcC/hvcC extracts the relevant parameter set NAL units - provided
>>> that they actually exist in the frame.
>>>
>>> Some encoders don't provide split out extradata directly on init (or
>>> at all). In particular, the MediaFoundation encoder wrapper doesn't
>>> always (depending on the actual encoder device) - this is the case for
>>> Qualcomm's HEVC encoder on SD835, and also on some QSV H264 encoders).
>>>
>>> This only works for cases where the moov hasn't already been written
>>> (e.g. when not writing fragmented mp4 with empty_moov, unless using
>>> the delay_moov option).
>>>
>>> Signed-off-by: Martin Storsjö
>>> ---
>>> libavformat/movenc.c | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>>> index 27d7621e27..6a85440a3f 100644
>>> --- a/libavformat/movenc.c
>>> +++ b/libavformat/movenc.c
>>> @@ -5584,7 +5584,9 @@ int ff_mov_write_packet(AVFormatContext *s,
>>> AVPacket *pkt)
>>>
>>> if ((par->codec_id == AV_CODEC_ID_DNXHD ||
>>> par->codec_id == AV_CODEC_ID_TRUEHD ||
>>> - par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len) {
>>> + par->codec_id == AV_CODEC_ID_AC3 ||
>>> + par->codec_id == AV_CODEC_ID_H264 ||
>>> + par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
>>> /* copy frame to create needed atoms */
>>> trk->vos_len = size;
>>> trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
>>
>> This changes avcintra output
>>
>> example testcase:
>> ./ffmpeg -i ~/videos/mm-short.mpg -avcintra-class 100 -tune psnr
>> -flags +ildct-global_header -t 0.5 -pix_fmt yuv422p10 -vf
>> scale=1920:1080 -an file.mov
>
> Right, if explicitly disabling global headers on the encoder, this will
> end up putting them back.
>
> Any ideas on what the best path forward would be?
>
> // Martin
If I am not mistaken, then the sample created by the above command line
is annex b in mp4 which is against the spec. So changing the output is
nothing to worry about.
- 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 v2] movenc: Use first H264/HEVC frame as extradata, if it is missing
On Thu, 21 May 2020, Michael Niedermayer wrote:
On Wed, May 20, 2020 at 09:10:21AM +0300, Martin Storsjö wrote:
Sticking a full frame in the extradata works, as the code for writing
the avcC/hvcC extracts the relevant parameter set NAL units - provided
that they actually exist in the frame.
Some encoders don't provide split out extradata directly on init (or
at all). In particular, the MediaFoundation encoder wrapper doesn't
always (depending on the actual encoder device) - this is the case for
Qualcomm's HEVC encoder on SD835, and also on some QSV H264 encoders).
This only works for cases where the moov hasn't already been written
(e.g. when not writing fragmented mp4 with empty_moov, unless using
the delay_moov option).
Signed-off-by: Martin Storsjö
---
libavformat/movenc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index 27d7621e27..6a85440a3f 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -5584,7 +5584,9 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket *pkt)
if ((par->codec_id == AV_CODEC_ID_DNXHD ||
par->codec_id == AV_CODEC_ID_TRUEHD ||
- par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len) {
+ par->codec_id == AV_CODEC_ID_AC3 ||
+ par->codec_id == AV_CODEC_ID_H264 ||
+ par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
/* copy frame to create needed atoms */
trk->vos_len = size;
trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
This changes avcintra output
example testcase:
./ffmpeg -i ~/videos/mm-short.mpg -avcintra-class 100 -tune psnr -flags
+ildct-global_header -t 0.5 -pix_fmt yuv422p10 -vf scale=1920:1080 -an file.mov
Right, if explicitly disabling global headers on the encoder, this will
end up putting them back.
Any ideas on what the best path forward would be?
// 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] cbs_h265: Fix use of an uninitialized variable
This fixes test failures in fate-cbs-hevc-* in certain configurations
since c53f9f436440be4e18.
---
Not sure if this is what was intended, but the current code doesn't
work at least.
---
libavcodec/cbs_h265_syntax_template.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavcodec/cbs_h265_syntax_template.c
b/libavcodec/cbs_h265_syntax_template.c
index 55da1a0a11..a3c1d1c7ca 100644
--- a/libavcodec/cbs_h265_syntax_template.c
+++ b/libavcodec/cbs_h265_syntax_template.c
@@ -552,7 +552,7 @@ static int FUNC(st_ref_pic_set)(CodedBitstreamContext *ctx,
RWContext *rw,
flags(use_delta_flag[j], 1, j);
else
infer(use_delta_flag[j], 1);
-if (current->use_delta_flag[i])
+if (current->use_delta_flag[j])
++num_ref_pics;
}
if (num_ref_pics >= HEVC_MAX_DPB_SIZE) {
--
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 v2] movenc: Use first H264/HEVC frame as extradata, if it is missing
On Wed, May 20, 2020 at 09:10:21AM +0300, Martin Storsjö wrote:
> Sticking a full frame in the extradata works, as the code for writing
> the avcC/hvcC extracts the relevant parameter set NAL units - provided
> that they actually exist in the frame.
>
> Some encoders don't provide split out extradata directly on init (or
> at all). In particular, the MediaFoundation encoder wrapper doesn't
> always (depending on the actual encoder device) - this is the case for
> Qualcomm's HEVC encoder on SD835, and also on some QSV H264 encoders).
>
> This only works for cases where the moov hasn't already been written
> (e.g. when not writing fragmented mp4 with empty_moov, unless using
> the delay_moov option).
>
> Signed-off-by: Martin Storsjö
> ---
> libavformat/movenc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 27d7621e27..6a85440a3f 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -5584,7 +5584,9 @@ int ff_mov_write_packet(AVFormatContext *s, AVPacket
> *pkt)
>
> if ((par->codec_id == AV_CODEC_ID_DNXHD ||
> par->codec_id == AV_CODEC_ID_TRUEHD ||
> - par->codec_id == AV_CODEC_ID_AC3) && !trk->vos_len) {
> + par->codec_id == AV_CODEC_ID_AC3 ||
> + par->codec_id == AV_CODEC_ID_H264 ||
> + par->codec_id == AV_CODEC_ID_HEVC) && !trk->vos_len) {
> /* copy frame to create needed atoms */
> trk->vos_len = size;
> trk->vos_data = av_malloc(size + AV_INPUT_BUFFER_PADDING_SIZE);
This changes avcintra output
example testcase:
./ffmpeg -i ~/videos/mm-short.mpg -avcintra-class 100 -tune psnr -flags
+ildct-global_header -t 0.5 -pix_fmt yuv422p10 -vf scale=1920:1080 -an file.mov
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Avoid a single point of failure, be that a person or equipment.
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] Use gcc/clang builtins for av_sat_(add|sub)_64_c if available.
On Wed, May 20, 2020 at 01:02:39PM -0700, Dale Curtis wrote:
> On Wed, May 20, 2020 at 1:17 AM Michael Niedermayer
> wrote:
>
> >
> > In file included from ./libavutil/avutil.h:296:0,
> > from ./libavutil/log.h:25,
> > from ./libavutil/thread.h:34,
> > from libavdevice/alldevices.c:22:
> > ./libavutil/common.h: In function ‘av_sat_add64_c’:
> > ./libavutil/common.h:302:105: error: missing binary operator before token
> > "("
> > #if (!defined(__INTEL_COMPILER) && AV_GCC_VERSION_AT_LEAST(5,1)) ||
> > (defined(__clang__) && __has_builtin(__builtin_add_overflow))
> >
> >
> What compiler is this? This is a seriously ancient function, so surprised
> it's not present, but a && __has_builtin would fix this.
gcc (Ubuntu 7.5.0-3ubuntu1~18.04) 7.5.0
used with ccache on a x86-64
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The real ebay dictionary, page 1
"Used only once"- "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
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] Specifying MOV_TIMESCALE from a command-line switch
On Thu, 21 May 2020, Kaplan, John wrote: Hi Guys, I've just finished a bunch of research on encoding audio .flac files into MP4/AAC using ffmpeg, and we're seeing a detail in the header data that is keeping some audio players from correctly interpreting the metadata in the MP4 elst box. Long story short, the issue is that MOV_TIMESCALE is hard-coded to 1000, which sets the mvhd box timescale to 1000. The trouble is that in an audio file context, the expected default for that value is the same as the track timescale, which is the sample rate, i.e. usually 44100. That 1000 value seems valid according to the MP4 spec as far as I can tell, but some commercial audio players, including the latest Apple player, don't interpret it correctly and play audio with gaps between those files rather than gapless playback as expected. We've experimented with setting that variable to 44100 in a local ffmpeg build, and that change is all it takes to change the mvhd box timescale value, and ffmpeg reacts to that by putting the expected values into the elist metadata. So Has anybody proposed a patch to allow people to set that MOV_TIMESCALE variable from the command line? It seems that would be an easy patch to do and could be used to fix this and numerous other requests for different movie timescales. I looked for discussion on this and didn't find mention in bugs, but found this thread: http://www.ffmpeg-archive.org/Quicktime-Specify-a-Movie-Header-timescale-of-24000-tt4683020.html#none ..not sure if this has been discussed elsewhere and I missed it. I'd be willing to do the work if the team agrees this is a good idea, or if there is another proposal that would have the same result. https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=975 There is even some code which automatically determines time scale, but it seems to limit it to 1? Shouldn't we simply set the MOV timescale as the least common multiple of the track timescales? Regards, Marton ___ 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/tiff: Check for Tiled and Stripped TIFFs
On Thu, Feb 27, 2020 at 12:07:16PM -0300, James Almer wrote:
> On 2/27/2020 11:10 AM, Carl Eugen Hoyos wrote:
> > Am Do., 27. Feb. 2020 um 15:05 Uhr schrieb Anton Khirnov
> > :
> >>
> >> Quoting Michael Niedermayer (2020-02-19 16:49:51)
> >>> TIFF 6 spec: "Do not use both strip-oriented and tile-oriented fields in
> >>> the same TIFF file."
> >>>
> >>> Fixes: null pointer use, crash
> >>> Fixes: crash-762680f9d1b27f9b9085e12887ad44893fb2b020
> >>>
> >>> Found-by: Shiziru
> >>> Signed-off-by: Michael Niedermayer
> >>> ---
> >>> libavcodec/tiff.c | 6 ++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> >>> index e8357114de..fd50b1cbfa 100644
> >>> --- a/libavcodec/tiff.c
> >>> +++ b/libavcodec/tiff.c
> >>> @@ -1873,6 +1873,12 @@ again:
> >>> return AVERROR_INVALIDDATA;
> >>> }
> >>>
> >>> +if ( (s->is_tiled || s->tile_byte_counts_offset ||
> >>> s->tile_offsets_offset || s->tile_width || s->tile_length ||
> >>> s->tile_count)
> >>> +&& (s->strippos || s->strips || s->stripoff || s->rps || s->sot
> >>> || s->sstype || s->stripsize || s->stripsizesoff)) {
> >>
> >> This is horribly unreadable. Putting those checks into macros, like
> >> HAVE_TILES(s) and HAVE_STRIPS(s) would make it a lot better.
> >
> > Were else in the file could the macros be used?
> > I fear that adding macros that are only used in one place will make the
> > code much more unreadable.
>
> I don't agree. An "if (HAVE_TILES(s) && HAVE_STRIPS(s))" check is easy
> to understand at first glance, regardless of the internals of each of
> those macros. It achieves the same effect as adding a comment in the
> code to explain what all those checks do.
>
> See IS_IRAP_NAL(nal) and IS_IDR_NAL(nal) in hevc_parser for another
> example of this simplification, which are also used in a single place.
I dont see IS_IRAP_NAL() and IS_IDR_NAL() to check every field in the headers
to be consistent with IDR / IRAP NALs, which is what the tiff code in this
thread does kind off ..
to match hevc, i could add something like
#define HAVE_TILES(s) s->has_tiles
#define HAVE_STRIPS(s) s->strips
but iam not sure what that would simplify
The validity checks are needed once and if thats put in a macro then maybe that
could be called HAVE_RANDOM_TILE_BITS() and HAVE_RANDOM_STRIP_BITS() could be
maybe a possibility
then again if thats what is suggested then i would suggest to rather add 2
local variable with these names instead of a macro which would seem more
readable
whats your opinion ?
Thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who are too smart to engage in politics are punished by being
governed by those who are dumber. -- Plato
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".
