Re: [FFmpeg-devel] [PATCH] Issue#6001:Fix for incorrect refs frame count in ffprobe

2019-07-18 Thread Hendrik Leppkes
On Thu, Jul 18, 2019 at 5:33 AM Mukund Manikarnike  wrote:
>
> Hi,
>
> Please find the patch for issue#6001 attached.

The patch is not correct. As you may have noticed,
ff_h264_decode_seq_parameter_set does not set *any* parameters in
avctx, that is because decoding a SPS and actually activating and
using the SPS are seperate actions. As such, this is the wrong spot to
do this.
The SPS values should be set in the avctx in the place where the  SPS
is activated for decoding image data, not when the SPS itself is being
decoded.

This would probably be something like h264_init_ps, which does appear
to already set avctx->refs.

- Hendrik
___
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] matroskadec: Add sizes to forward declarations

2019-07-18 Thread Hendrik Leppkes
On Wed, Jul 17, 2019 at 5:35 AM Andreas Rheinhardt
 wrote:
>
> Unknown-length elements end when an element not allowed in them, but
> allowed at a higher level is encountered. In order to check for this,
> c1abd95a added a pointer to every syntax level's parent to each
> EbmlSyntax. Given that the parent must of course also reference the
> child in order to be able to enter said child level, one needs to use
> forward declarations.
> These forward declarations constitute tentative definitions and tentative
> definitions with internal linkage (like our syntaxes) must not be an
> incomplete type. Yet they were an incomplete type and while GCC and
> Clang did not even warn about this (on default warning levels), it
> broke compilation with MSVC. Therefore this commit adds the sizes.
>
> Signed-off-by: Andreas Rheinhardt 
> ---
> 1. I am terribly sorry for this. I did test with both GCC and Clang, but
> not with -pedantic, but only with default warning levels.
> 2. I don't have an MSVC setup here, so could please someone confirm that
> this patch indeed fixes the compilation? All I know is that GCC's
> -pedantic warnings are gone with this patch and that the requirement
> that tentative definitions with internal linkage must not be of
> incomplete type is fulfilled now.
>
>  libavformat/matroskadec.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index b97189e674..fb0356e7b7 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -389,12 +389,16 @@ typedef struct MatroskaDemuxContext {
>
>  #define CHILD_OF(parent) { .def = { .n = parent } }
>
> -static const EbmlSyntax ebml_syntax[], matroska_segment[], 
> matroska_track_video_color[], matroska_track_video[],
> -matroska_track[], matroska_track_encoding[], 
> matroska_track_encodings[],
> -matroska_track_combine_planes[], 
> matroska_track_operation[], matroska_tracks[],
> -matroska_attachments[], matroska_chapter_entry[], 
> matroska_chapter[], matroska_chapters[],
> -matroska_index_entry[], matroska_index[], 
> matroska_tag[], matroska_tags[], matroska_seekhead[],
> -matroska_blockadditions[], matroska_blockgroup[], 
> matroska_cluster_parsing[];
> +// The following forward declarations need their size because
> +// a tentative definition with internal linkage must not be an
> +// incomplete type (6.7.2 in C90, 6.9.2 in C99).
> +// Removing the sizes breaks MSVC.
> +static const EbmlSyntax ebml_syntax[3], matroska_segment[9], 
> matroska_track_video_color[15], matroska_track_video[19],
> +matroska_track[27], matroska_track_encoding[6], 
> matroska_track_encodings[2],
> +matroska_track_combine_planes[2], 
> matroska_track_operation[2], matroska_tracks[2],
> +matroska_attachments[2], matroska_chapter_entry[9], 
> matroska_chapter[6], matroska_chapters[2],
> +matroska_index_entry[3], matroska_index[2], 
> matroska_tag[3], matroska_tags[2], matroska_seekhead[2],
> +matroska_blockadditions[2], matroska_blockgroup[8], 
> matroska_cluster_parsing[8];
>
>  static const EbmlSyntax ebml_header[] = {
>  { EBML_ID_EBMLREADVERSION,EBML_UINT, 0, offsetof(Ebml, version), 
> { .u = EBML_VERSION } },


Fixes compilaton on MSVC and FATE passes. Will apply soon if noone
beats me to it.

As an additional note, this section still causes a lot of warnings
because "const" is repeated. The typedef for EbmlSyntax is already
const, so this evaluates to "const const" and makes it throw warnings
about that. Maybe you can remove one of those in a follow up?

- Hendrik
___
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] how to create sub project / git repo under FFMPEG

2019-07-18 Thread Guo, Yejun


> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Michael Niedermayer
> Sent: Tuesday, July 16, 2019 5:00 PM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] how to create sub project / git repo under FFMPEG
> 
> On Tue, Jul 16, 2019 at 06:07:58AM +, Guo, Yejun wrote:
> > Hi Michael,
> >
> > With the development of ffmpeg dnn module, there is a need to get a git 
> > repo,
> controlled by FFMPEG, to hold dnn materials, such as training scripts, trained
> parameters etc, as discussed at
> http://ffmpeg.org/pipermail/ffmpeg-devel/2019-June/245750.html.
> >
> > Could you please let me know how to create a sub project / git repo under
> FFMPEG? thanks.
> 
> I can create a git repo on git.ffmpeg.org assuming there is consensus that
> one should be created as well as a name for it
> 
> One alternative is to use github or to ask jb about creating one on
> videolan
> 
> Just depends on whatever people prefer ?

I personally prefer github to host the repo, the repo name might be 
ffmpeg_dnn_material or
ffmpeg_dnn_data, or something others. I'm open to the name, just up to you.

My plan is to first ask authors of filter vf_sr and vf_derain to move their dnn 
material to this new
repo from their personal repos. (see personal repo at 
https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/doc/filters.texi#l8370
and 
https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/doc/filters.texi#l16718) 

So, once there is a need to update these dnn material, FFMPEG community can 
accept the pull request, instead of waiting for the authors who might ignore it 
for a long time.

And for future dnn relative developments, the dnn material will go to this repo.

> 
> Thanks
> 
> [...]
> 
> --
> Michael GnuPG fingerprint:
> 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> "You are 36 times more likely to die in a bathtub than at the hands of a
> terrorist. Also, you are 2.5 times more likely to become a president and
> 2 times more likely to become an astronaut, than to die in a terrorist
> attack." -- Thoughty2

___
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 2/2] doc/ffmpeg.texi: update docs for autoscale/autorotate

2019-07-18 Thread Fu, Linjie
> -Original Message-
> From: Eoff, Ullysses A
> Sent: Wednesday, July 17, 2019 21:48
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Cc: Fu, Linjie 
> Subject: RE: [FFmpeg-devel] [PATCH, v2 2/2] doc/ffmpeg.texi: update docs
> for autoscale/autorotate
> 
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf Of Linjie Fu
> > Sent: Tuesday, July 16, 2019 4:20 AM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Fu, Linjie 
> > Subject: [FFmpeg-devel] [PATCH, v2 2/2] doc/ffmpeg.texi: update docs for
> autoscale/autorotate
> >
> > Add docs for autoscale.
> >
> > Update information for autorotate according to ffplay.
> >
> > Signed-off-by: Linjie Fu 
> > ---
> >  doc/ffmpeg.texi | 14 ++
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > index cd35eb49c8..b91da2b2b4 100644
> > --- a/doc/ffmpeg.texi
> > +++ b/doc/ffmpeg.texi
> > @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
> >  Technical note -- attachments are implemented as codec extradata, so this
> >  option can actually be used to extract extradata from any stream, not just
> >  attachments.
> > -
> > -@item -noautorotate
> > -Disable automatically rotating video based on file metadata.
> > -
> >  @end table
> >
> >  @section Video Options
> > @@ -819,6 +815,16 @@ Create the filtergraph specified by
> @var{filtergraph} and use it to
> >  filter the stream.
> >
> >  This is an alias for @code{-filter:v}, see the @ref{filter_option,,-filter
> option}.
> > +
> > +@item -autorotate
> > +Automatically rotate the video according to file metadata. Enabled by
> > +default, use @option{-noautorotate} to disable it.
> > +
> > +@item -autoscale
> > +Automatically scale the video according to the resolution of first frame.
> > +Enabled by default, use @option{-noautoscale} to disable it. Each frame
> of
> > +the output raw video can be in different resolutions and is in need to be
> > +handled next.
> 
> This last sentence has strange grammar.  Here's my shot at it:
> 
> "When autoscale is disabled, all output frames might not be in the
> same resolution and may require some additional explicit processing
> according to your final rendering/output destination."
> 

This sounds much better, thanks a lot.
Will update.

- linjie


___
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 2/2] doc/ffmpeg.texi: update docs for autoscale/autorotate

2019-07-18 Thread Fu, Linjie
> -Original Message-
> From: Nicolas George [mailto:geo...@nsup.org]
> Sent: Thursday, July 18, 2019 03:07
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Cc: Fu, Linjie 
> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] doc/ffmpeg.texi: update docs
> for autoscale/autorotate
> 
> Linjie Fu (12019-07-16):
> > Add docs for autoscale.
> >
> > Update information for autorotate according to ffplay.
> >
> > Signed-off-by: Linjie Fu 
> > ---
> >  doc/ffmpeg.texi | 14 ++
> >  1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > index cd35eb49c8..b91da2b2b4 100644
> > --- a/doc/ffmpeg.texi
> > +++ b/doc/ffmpeg.texi
> > @@ -734,10 +734,6 @@ ffmpeg -dump_attachment:t "" -i INPUT
> >  Technical note -- attachments are implemented as codec extradata, so this
> >  option can actually be used to extract extradata from any stream, not just
> >  attachments.
> > -
> > -@item -noautorotate
> > -Disable automatically rotating video based on file metadata.
> > -
> >  @end table
> >
> >  @section Video Options
> > @@ -819,6 +815,16 @@ Create the filtergraph specified by
> @var{filtergraph} and use it to
> >  filter the stream.
> >
> >  This is an alias for @code{-filter:v}, see the @ref{filter_option,,-filter
> option}.
> > +
> > +@item -autorotate
> > +Automatically rotate the video according to file metadata. Enabled by
> > +default, use @option{-noautorotate} to disable it.
> > +
> 
> > +@item -autoscale
> > +Automatically scale the video according to the resolution of first frame.
> > +Enabled by default, use @option{-noautoscale} to disable it. Each frame
> of
> > +the output raw video can be in different resolutions and is in need to be
> > +handled next.
> 
> Since this is completely hackish in lavfi, I think it needs to warn the
> user much more strongly to expect problems.

Yes, will update as Artie had suggested if no more comments:

When autoscale is disabled, all output frames might not be in the
same resolution and may require some additional explicit processing
according to your final rendering/output destination.
___
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] how to create sub project / git repo under FFMPEG

2019-07-18 Thread Jean-Baptiste Kempf
On Tue, Jul 16, 2019, at 11:00, Michael Niedermayer wrote:
> One alternative is to use github or to ask jb about creating one on
> videolan

It is trivial to create one either on the git.videolan.org or, better, to use 
the gitlab code.videolan.org/ffmpeg/ group, where you can create repos 
without asking anyone.

--
Jean-Baptiste Kempf - President
+33 672 704 734


___
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 2/2] doc/ffmpeg.texi: update docs for autoscale/autorotate

2019-07-18 Thread Nicolas George
Fu, Linjie (12019-07-18):
> Yes, will update as Artie had suggested if no more comments:
> 
> When autoscale is disabled, all output frames might not be in the
> same resolution and may require some additional explicit processing
> according to your final rendering/output destination.

That is not enough. You have to tell the user that this option will most
likely not work at all, and why.

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] how to create sub project / git repo under FFMPEG

2019-07-18 Thread Nicolas George
Guo, Yejun (12019-07-18):
> I personally prefer github to host the repo, the repo name might be
> ffmpeg_dnn_material or ffmpeg_dnn_data, or something others. I'm open
> to the name, just up to you.
> 
> My plan is to first ask authors of filter vf_sr and vf_derain to move
> their dnn material to this new repo from their personal repos. (see
> personal repo at
> https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/doc/filters.texi#l8370
> and 
> https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/doc/filters.texi#l16718) 
> 
> So, once there is a need to update these dnn material, FFMPEG
> community can accept the pull request, instead of waiting for the
> authors who might ignore it for a long time.

When you say "FFMPEG community can accept the pull request", you mean
into the official repository?

If so, then I strongly oppose anything like that: relying on GitHub or
any commercial / proprietary Git infrastructure, and depending on
web-based features.

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] how to create sub project / git repo under FFMPEG

2019-07-18 Thread Jean-Baptiste Kempf
On Thu, Jul 18, 2019, at 11:05, Nicolas George wrote:
> Guo, Yejun (12019-07-18):
> > I personally prefer github to host the repo, the repo name might be
> > ffmpeg_dnn_material or ffmpeg_dnn_data, or something others. I'm open
> > to the name, just up to you.
> > 
> > My plan is to first ask authors of filter vf_sr and vf_derain to move
> > their dnn material to this new repo from their personal repos. (see
> > personal repo at
> > https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/doc/filters.texi#l8370
> > and 
> > https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/HEAD:/doc/filters.texi#l16718)
> >  
> > 
> > So, once there is a need to update these dnn material, FFMPEG
> > community can accept the pull request, instead of waiting for the
> > authors who might ignore it for a long time.
> 
> When you say "FFMPEG community can accept the pull request", you mean
> into the official repository?
> 
> If so, then I strongly oppose anything like that: relying on GitHub or
> any commercial / proprietary Git infrastructure, and depending on
> web-based features.

Gitlab on videolan is the open source version, since we only rely on open 
source software for the whole infrastructure,
and you can use gitlab in CLI, and/or disable MR if you want (basically using 
it like a dumb git repo, if you want).

--
Jean-Baptiste Kempf - President
+33 672 704 734


___
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] lavc/phtread_frame: update hwaccel_priv_data in time for multithread

2019-07-18 Thread Linjie Fu
When resolution/format changes, hwaccel_uninit/hwaccel_init will
be called to destroy and re-create the hwaccel_priv_data. When output
frame number meets the constraints for vframes, the hwaccel_priv_data
modified in decoding thread won't be update to user-thread due to the
delay mechanism. It will lead to:
1. memory leak in child-thread.
2. double free in user-thread while calling avcodec_close().

Can be reproduced with a resolution change case, and use -vframes
to terminate the decode during dynamic resolution changing:

ffmpeg -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 -v verbose
-i ./reinit-large_420_8-to-small_420_8.h264 -pix_fmt nv12 -f rawvideo
-vsync passthrough -vframes 45 -y out.yuv

The root cause is the conflict between delay mechanism and -vframes.
FFmpeg won't output a frame if it's still receiving the initial packets,
so there is async between decode process and output. hwaccel_priv_data
in user thread won't be updated until the resolution changing
frame is output.

As user context should reflect the state of the last frame that
was output to the user, hwaccel_priv_data should be updated
separately from decoding thread in time.

Signed-off-by: Linjie Fu 
---
 libavcodec/pthread_frame.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index 36ac0ac..cf7a575 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -282,7 +282,6 @@ static int update_context_from_thread(AVCodecContext *dst, 
AVCodecContext *src,
 dst->sample_rate= src->sample_rate;
 dst->sample_fmt = src->sample_fmt;
 dst->channel_layout = src->channel_layout;
-dst->internal->hwaccel_priv_data = src->internal->hwaccel_priv_data;
 
 if (!!dst->hw_frames_ctx != !!src->hw_frames_ctx ||
 (dst->hw_frames_ctx && dst->hw_frames_ctx->data != 
src->hw_frames_ctx->data)) {
@@ -410,6 +409,7 @@ static int submit_packet(PerThreadContext *p, 
AVCodecContext *user_avctx,
 pthread_mutex_unlock(&prev_thread->progress_mutex);
 }
 
+p->avctx->internal->hwaccel_priv_data = 
prev_thread->avctx->internal->hwaccel_priv_data;
 err = update_context_from_thread(p->avctx, prev_thread->avctx, 0);
 if (err) {
 pthread_mutex_unlock(&p->mutex);
@@ -476,7 +476,7 @@ int ff_thread_decode_frame(AVCodecContext *avctx,
 FrameThreadContext *fctx = avctx->internal->thread_ctx;
 int finished = fctx->next_finished;
 PerThreadContext *p;
-int err;
+int err, cur_decoding;
 
 /* release the async lock, permitting blocked hwaccel threads to
  * go forward while we are in this function */
@@ -544,6 +544,13 @@ int ff_thread_decode_frame(AVCodecContext *avctx,
 
 if (fctx->next_decoding >= avctx->thread_count) fctx->next_decoding = 0;
 
+/* update hwaccel_priv_data from decoding thread */
+cur_decoding = fctx->next_decoding - 1;
+if (cur_decoding < 0) cur_decoding += avctx->thread_count;
+
+p = &fctx->threads[cur_decoding];
+avctx->internal->hwaccel_priv_data = p->avctx->internal->hwaccel_priv_data;
+
 fctx->next_finished = finished;
 
 /* return the size of the consumed packet if no error occurred */
-- 
2.7.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] [PATCH 2/2] avcodec/fitsdec: Prevent division by 0 with huge data_max

2019-07-18 Thread Michael Niedermayer
On Thu, Jul 18, 2019 at 08:21:21AM +0200, Reimar Döffinger wrote:
> 
> 
> On 16.07.2019, at 20:31, Michael Niedermayer  wrote:
> 
> > On Tue, Jul 16, 2019 at 08:34:14AM +0200, Reimar Döffinger wrote:
> >> On 16.07.2019, at 00:50, Michael Niedermayer  
> >> wrote:
> >> 
> >>> Fixes: division by 0
> >>> Fixes: 
> >>> 15657/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FITS_fuzzer-5738154838982656
> >>> 
> >>> Found-by: continuous fuzzing process 
> >>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> >>> Signed-off-by: Michael Niedermayer 
> >>> ---
> >>> libavcodec/fitsdec.c | 2 +-
> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>> 
> >>> diff --git a/libavcodec/fitsdec.c b/libavcodec/fitsdec.c
> >>> index 4f452422ef..fe941f199d 100644
> >>> --- a/libavcodec/fitsdec.c
> >>> +++ b/libavcodec/fitsdec.c
> >>> @@ -174,7 +174,7 @@ static int fits_read_header(AVCodecContext *avctx, 
> >>> const uint8_t **ptr, FITSHead
> >>>return AVERROR_INVALIDDATA;
> >>>}
> >>>av_log(avctx, AV_LOG_WARNING, "data min/max indicates a blank 
> >>> image\n");
> >>> -header->data_max ++;
> >>> +header->data_max += fabs(header->data_max) / 1000 + 1;
> >> 
> >> This is really non-obvious, both by itself, in where/how it causes the 
> >> division by 0 and that the error here isn't worse than the division by 0 
> >> for example, and why this constant was chosen.
> > 
> > division by 0 occured in:
> > *dst++ = ((t - header.data_min) * ((1 << (sizeof(type) * 8)) - 1)) / 
> > (header.data_max - header.data_min); \
> 
> I looked at the code, and now it makes even less sense to me.
> First, why is that reported as an error at all?
> Dividing by 0 is well defined for floating-point.

at least at the point where its stored in an integer it becomes painfull
to the compiler to find a way to do it.


> Next, your patch handles only one corner-case while not handling others.
> For example, data_min and data_max can also be inf or NaN, which equally make 
> no sense (and result in a division by NaN, which can hardly be better than a 
> division by 0).
> Next, bzero is applied to data_min and data_max which can cause precision 
> issues, so it's a bit questionable but maybe non-trivial to fix completely.
> However as data_max is never used but only the delta, most of these issues 
> can be fixed much more thoroughly (and improve performance) by calculating 
> and storing only that delta, and before applying bzero. Then delta can simply 
> be overridden to 1 if it is fishy (not a normal or 0).
> Of course there is a question if values above data_max are supposed to result 
> in output > 1 or be clamped, but I guess that issue can be ignored.
> As the code pays no particular attention to precision issue it would also be 
> a question if calculating the inverse and use multiplications instead of 
> divisions would make sense, but that admittedly is just an optimization.

Iam not sure if inf is a problem (from a very quick look) that would get
divided to 0 i guess nan would be an issue, i didnt think of this, i will 
redo this and post a better patch

Thnaks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Those who are best at talking, realize last or never when they are 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] FFmpeg classification

2019-07-18 Thread Thomas Volkert
On 17.07.2019 03:10, Maaya Murakami (JP) wrote:
> Hello
>
> Hope all is well.
> Currently I am researching about the trends of FFmpeg and its supporting
> codecs, ...

It seems that you are not the only one. During the last 24 hours I have
received 3 requests regarding only this topic, all coming from China.

Best regards.

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Issue#6001:Fix for incorrect refs frame count in ffprobe

2019-07-18 Thread Mukund Manikarnike
Hi Hendrik,

Thank you for pointing that out.
I'll look into that piece of code. It does appear to be getting set.
However, it doesn't reflect in what ffprobe shows in it's output.

Regards,
Mukund Manikarnike


On Thu, Jul 18, 2019 at 12:16 AM Hendrik Leppkes 
wrote:

> On Thu, Jul 18, 2019 at 5:33 AM Mukund Manikarnike 
> wrote:
> >
> > Hi,
> >
> > Please find the patch for issue#6001 attached.
>
> The patch is not correct. As you may have noticed,
> ff_h264_decode_seq_parameter_set does not set *any* parameters in
> avctx, that is because decoding a SPS and actually activating and
> using the SPS are seperate actions. As such, this is the wrong spot to
> do this.
> The SPS values should be set in the avctx in the place where the  SPS
> is activated for decoding image data, not when the SPS itself is being
> decoded.
>
> This would probably be something like h264_init_ps, which does appear
> to already set avctx->refs.
>
> - Hendrik
> ___
> 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 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 2/2] doc/ffmpeg.texi: update docs for autoscale/autorotate

2019-07-18 Thread Eoff, Ullysses A
> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of 
> Nicolas George
> Sent: Thursday, July 18, 2019 2:01 AM
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH, v2 2/2] doc/ffmpeg.texi: update docs for 
> autoscale/autorotate
> 
> Fu, Linjie (12019-07-18):
> > Yes, will update as Artie had suggested if no more comments:
> >
> > When autoscale is disabled, all output frames might not be in the
> > same resolution and may require some additional explicit processing
> > according to your final rendering/output destination.
> 
> That is not enough. You have to tell the user that this option will most
> likely not work at all, and why.
> 

@Nicolas, do you have any "examples" of what you might add to this?

Maybe something additional like this is enough:

Disabling autoscale may not work in all situations.  Therefore, it is not
recommended to disable it unless you really know what you are doing.
Disable autoscale at your own risk.

> Regards,
> 
> --
>   Nicolas George
___
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/ifv: Check for EOF in read_index()

2019-07-18 Thread Peter Ross
On Thu, Jul 18, 2019 at 01:42:23AM +0200, Michael Niedermayer wrote:
> Fixes: Timeout
> Fixes: 
> 15567/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5758451487080448
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/ifv.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/ifv.c b/libavformat/ifv.c
> index 6acbb29a75..f95e9b0e52 100644
> --- a/libavformat/ifv.c
> +++ b/libavformat/ifv.c
> @@ -68,6 +68,8 @@ static int read_index(AVFormatContext *s,
>  }
>  
>  for (i = start_index; i < end_index; i++) {
> +if (avio_feof(s->pb))
> +return AVERROR_EOF;
>  pos = avio_rl32(s->pb);
>  size = avio_rl32(s->pb);
>  

i approve.

-- Peter
(A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B)


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] fftools/ffmpeg_filter: add -autoscale to disable/enable the default scale

2019-07-18 Thread Eoff, Ullysses A
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of 
> Michael Niedermayer
> On Mon, Jul 15, 2019 at 06:38:35PM +0800, Linjie Fu wrote:
> > +{ "autoscale",HAS_ARG | OPT_BOOL | OPT_SPEC |
> > +  OPT_EXPERT | OPT_INPUT,  
> >   { .off = OFFSET(autoscale) },
> > +"automatically insert correct scale filters" },
> 
> I think this description is inadequate to understand what the option does.
> Scale filters are inserted at various places (for example within a filter
> graph). This option here affects a specific case.
> 

Would this be sufficient?

"automatically insert a scale filter to scale the output of all frames to
  match the resolution of the initial frame"

If not, any other suggestions?

Thanks,
U. Artie
___
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] Setup for extracting quantization parameters from encoded streams

2019-07-18 Thread Moritz Barsnick
On Wed, Jul 17, 2019 at 16:18:11 -0700, Juan De León wrote:
> ---
>  libavfilter/vf_extractqp.c  | 116 
>  libavutil/Makefile  |   2 +
>  libavutil/quantization_params.c |  28 
>  libavutil/quantization_params.h |  98 +++
>  4 files changed, 244 insertions(+)
>  create mode 100644 libavfilter/vf_extractqp.c
>  create mode 100644 libavutil/quantization_params.c
>  create mode 100644 libavutil/quantization_params.h

Is this just an initial proposal and a proof of concept?

The side data which is supposed to come from the codecs isn't available
yet. I don't see AV_FRAME_DATA_QUANTIZATION_PARAMS defined anywhere. I
see no example of how to let the decoders detect the quantization and
insert that side data.

The "extractqp" filter apparently demonstrates how to detect
availability of this side data, i.e. shows how to call
av_frame_get_side_data(). It doesn't do anything useful at all though,
and it isn't included in the build (or documented).

Moritz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] Setup for extracting quantization parameters from encoded streams

2019-07-18 Thread Lynne
Jul 18, 2019, 12:25 AM by juandl-at-google@ffmpeg.org 
:

> For context
> https://docs.google.com/document/d/1WClt3EqhjwdGXhEw386O0wfn3IBQ1Ib-_5emVM1gbnA/edit?usp=sharing
>  
> 
> Feel free to comment the doc.
>

This is meant for analyzers and debugging, so I don't think we should implement 
such tools.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH] avformat/mxfenc: fix index byte count in partition header

2019-07-18 Thread Baptiste Coudurier
---
 libavformat/mxfenc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index b677f6af8e..2e54320cf0 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -1944,8 +1944,7 @@ static int mxf_write_partition(AVFormatContext *s, int 
bodysid,
 index_byte_count = 80;
 
 if (index_byte_count) {
-// add encoded ber length
-index_byte_count += 16 + klv_ber_length(index_byte_count);
+index_byte_count += 16 + 4; // add encoded ber4 length
 index_byte_count += klv_fill_size(index_byte_count);
 }
 
-- 
2.20.1 (Apple Git-117)

___
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] matroskadec: Remove redundant const

2019-07-18 Thread Andreas Rheinhardt
The typedef used to define EbmlSyntax already includes a const qualifier
so that it is unnecessary to include another const qualifier in future
definitions and declarations. Given that MSVC warns about this, this
commit removes these redundant const qualifiers.

Suggested-by: Hendrik Leppkes 
Signed-off-by: Andreas Rheinhardt 
---
 libavformat/matroskadec.c | 92 +++
 1 file changed, 46 insertions(+), 46 deletions(-)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index fb0356e7b7..4e20f15792 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -393,14 +393,14 @@ typedef struct MatroskaDemuxContext {
 // a tentative definition with internal linkage must not be an
 // incomplete type (6.7.2 in C90, 6.9.2 in C99).
 // Removing the sizes breaks MSVC.
-static const EbmlSyntax ebml_syntax[3], matroska_segment[9], 
matroska_track_video_color[15], matroska_track_video[19],
-matroska_track[27], matroska_track_encoding[6], 
matroska_track_encodings[2],
-matroska_track_combine_planes[2], 
matroska_track_operation[2], matroska_tracks[2],
-matroska_attachments[2], matroska_chapter_entry[9], 
matroska_chapter[6], matroska_chapters[2],
-matroska_index_entry[3], matroska_index[2], 
matroska_tag[3], matroska_tags[2], matroska_seekhead[2],
-matroska_blockadditions[2], matroska_blockgroup[8], 
matroska_cluster_parsing[8];
-
-static const EbmlSyntax ebml_header[] = {
+static EbmlSyntax ebml_syntax[3], matroska_segment[9], 
matroska_track_video_color[15], matroska_track_video[19],
+  matroska_track[27], matroska_track_encoding[6], 
matroska_track_encodings[2],
+  matroska_track_combine_planes[2], 
matroska_track_operation[2], matroska_tracks[2],
+  matroska_attachments[2], matroska_chapter_entry[9], 
matroska_chapter[6], matroska_chapters[2],
+  matroska_index_entry[3], matroska_index[2], matroska_tag[3], 
matroska_tags[2], matroska_seekhead[2],
+  matroska_blockadditions[2], matroska_blockgroup[8], 
matroska_cluster_parsing[8];
+
+static EbmlSyntax ebml_header[] = {
 { EBML_ID_EBMLREADVERSION,EBML_UINT, 0, offsetof(Ebml, version),   
  { .u = EBML_VERSION } },
 { EBML_ID_EBMLMAXSIZELENGTH,  EBML_UINT, 0, offsetof(Ebml, max_size),  
  { .u = 8 } },
 { EBML_ID_EBMLMAXIDLENGTH,EBML_UINT, 0, offsetof(Ebml, id_length), 
  { .u = 4 } },
@@ -411,13 +411,13 @@ static const EbmlSyntax ebml_header[] = {
 CHILD_OF(ebml_syntax)
 };
 
-static const EbmlSyntax ebml_syntax[] = {
+static EbmlSyntax ebml_syntax[] = {
 { EBML_ID_HEADER,  EBML_NEST, 0, 0, { .n = ebml_header } },
 { MATROSKA_ID_SEGMENT, EBML_STOP },
 { 0 }
 };
 
-static const EbmlSyntax matroska_info[] = {
+static EbmlSyntax matroska_info[] = {
 { MATROSKA_ID_TIMECODESCALE, EBML_UINT,  0, offsetof(MatroskaDemuxContext, 
time_scale), { .u = 100 } },
 { MATROSKA_ID_DURATION,  EBML_FLOAT, 0, offsetof(MatroskaDemuxContext, 
duration) },
 { MATROSKA_ID_TITLE, EBML_UTF8,  0, offsetof(MatroskaDemuxContext, 
title) },
@@ -428,7 +428,7 @@ static const EbmlSyntax matroska_info[] = {
 CHILD_OF(matroska_segment)
 };
 
-static const EbmlSyntax matroska_mastering_meta[] = {
+static EbmlSyntax matroska_mastering_meta[] = {
 { MATROSKA_ID_VIDEOCOLOR_RX, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, r_x), { .f=-1 } },
 { MATROSKA_ID_VIDEOCOLOR_RY, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, r_y), { .f=-1 } },
 { MATROSKA_ID_VIDEOCOLOR_GX, EBML_FLOAT, 0, 
offsetof(MatroskaMasteringMeta, g_x), { .f=-1 } },
@@ -442,7 +442,7 @@ static const EbmlSyntax matroska_mastering_meta[] = {
 CHILD_OF(matroska_track_video_color)
 };
 
-static const EbmlSyntax matroska_track_video_color[] = {
+static EbmlSyntax matroska_track_video_color[] = {
 { MATROSKA_ID_VIDEOCOLORMATRIXCOEFF,  EBML_UINT, 0, 
offsetof(MatroskaTrackVideoColor, matrix_coefficients), { .u = 
AVCOL_SPC_UNSPECIFIED } },
 { MATROSKA_ID_VIDEOCOLORBITSPERCHANNEL,   EBML_UINT, 0, 
offsetof(MatroskaTrackVideoColor, bits_per_channel), { .u=0 } },
 { MATROSKA_ID_VIDEOCOLORCHROMASUBHORZ,EBML_UINT, 0, 
offsetof(MatroskaTrackVideoColor, chroma_sub_horz), { .u=0 } },
@@ -460,7 +460,7 @@ static const EbmlSyntax matroska_track_video_color[] = {
 CHILD_OF(matroska_track_video)
 };
 
-static const EbmlSyntax matroska_track_video_projection[] = {
+static EbmlSyntax matroska_track_video_projection[] = {
 { MATROSKA_ID_VIDEOPROJECTIONTYPE,EBML_UINT,  0, 
offsetof(MatroskaTrackVideoProjection, type), { .u = 
MATROSKA_VIDEO_PROJECTION_TYPE_RECTANGULAR } },
 { MATROSKA_ID_VIDEOPROJECTIONPRIVATE, EBML_BIN,   0, 
offsetof(MatroskaTrackVideoProjection, private) },
 { MATROSKA_ID_VIDEOPROJECTIONPOSEYAW, EBML_FLOAT, 0, 
offsetof(MatroskaTrackVi

Re: [FFmpeg-devel] [PATCH v1] Bug #8027 - Wrong result for FFSIGN(0)

2019-07-18 Thread Ulf Zibis

Am 17.07.19 um 10:12 schrieb Hendrik Leppkes:
> ..., the macro was defined the way it is, and changing it now has a
> chance of breaking
> random places that rely on the current behavior.

OK, that would be risky.

Maybe we could add another macro FFSGN. Then maintainers of individual
code parts could check the usage over the time.

#define FFSGN(a) ((a) > 0 ? 1 : (a) < 0 ? -1 : 0)

-Ulf

___
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 v1] Bug #8027 - Wrong result for FFSIGN(0)

2019-07-18 Thread Nicolas George
Ulf Zibis (12019-07-18):
> Maybe we could add another macro FFSGN. Then maintainers of individual
> code parts could check the usage over the time.

Or maybe write robust code that does not need that kind of hair
splitting.

Regards,

-- 
  Nicolas George


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH] avformat/ifv: Check for EOF in read_index()

2019-07-18 Thread Michael Niedermayer
On Fri, Jul 19, 2019 at 12:32:06AM +1000, Peter Ross wrote:
> On Thu, Jul 18, 2019 at 01:42:23AM +0200, Michael Niedermayer wrote:
> > Fixes: Timeout
> > Fixes: 
> > 15567/clusterfuzz-testcase-minimized-ffmpeg_DEMUXER_fuzzer-5758451487080448
> > 
> > Found-by: continuous fuzzing process 
> > https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > Signed-off-by: Michael Niedermayer 
> > ---
> >  libavformat/ifv.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/libavformat/ifv.c b/libavformat/ifv.c
> > index 6acbb29a75..f95e9b0e52 100644
> > --- a/libavformat/ifv.c
> > +++ b/libavformat/ifv.c
> > @@ -68,6 +68,8 @@ static int read_index(AVFormatContext *s,
> >  }
> >  
> >  for (i = start_index; i < end_index; i++) {
> > +if (avio_feof(s->pb))
> > +return AVERROR_EOF;
> >  pos = avio_rl32(s->pb);
> >  size = avio_rl32(s->pb);
> >  
> 
> i approve.

will apply

thanks

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway


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] libavformat/subfile: Fix SEEK_CUR and SEEK_END seeking

2019-07-18 Thread Michael Niedermayer
On Wed, Jul 17, 2019 at 07:45:00PM +, Andreas Rheinhardt wrote:
> Michael Niedermayer:
> > On Mon, Jul 15, 2019 at 07:48:35PM +0200, Andreas Rheinhardt wrote:
> >> Up until now, when performing a SEEK_END seek, the subfile protocol
> >> ignored the desired position (relative to EOF) and used the current
> >> absolute offset in the input file instead.
> > 
> > I think this comment is only intended to be in the previous patch
> > 
> > [...]
> No, the new patch supersedes the previous patch and therefore the
> commit message explains what is wrong with SEEK_END as well as with
> SEEK_CUR. Or do you want the changes to be split in two commits?

ahh, ok, sorry, i had looked at the code after locally applying
both which worked and caused the hunk to be not in the 2nd
so what i looked at was different from what you posted to the ML

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The worst form of inequality is to try to make unequal things equal.
-- 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/2] fftools/ffmpeg_filter: add -autoscale to disable/enable the default scale

2019-07-18 Thread Michael Niedermayer
On Thu, Jul 18, 2019 at 02:44:46PM +, Eoff, Ullysses A wrote:
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of 
> > Michael Niedermayer
> > On Mon, Jul 15, 2019 at 06:38:35PM +0800, Linjie Fu wrote:
> > > +{ "autoscale",HAS_ARG | OPT_BOOL | OPT_SPEC |
> > > +  OPT_EXPERT | OPT_INPUT,
> > > { .off = OFFSET(autoscale) },
> > > +"automatically insert correct scale filters" },
> > 
> > I think this description is inadequate to understand what the option does.
> > Scale filters are inserted at various places (for example within a filter
> > graph). This option here affects a specific case.
> > 
> 
> Would this be sufficient?
> 
> "automatically insert a scale filter to scale the output of all frames to
>   match the resolution of the initial frame"

this sounds better, yes


> 
> If not, any other suggestions?

maybe it could say how this relates to the filter graph/chain. 
the user doenst know from this if this occurs before or after the
filter graph.


Also what happens when a encoder/muxer does not support this ?

Thanks

[...]
-- 
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] matroskadec: Remove redundant const

2019-07-18 Thread Carl Eugen Hoyos
Am Do., 18. Juli 2019 um 21:15 Uhr schrieb Andreas Rheinhardt
:

> The typedef used to define EbmlSyntax already includes a const qualifier
> so that it is unnecessary to include another const qualifier in future
> definitions and declarations. Given that MSVC warns about this, this
> commit removes these redundant const qualifiers.

I applied this patch.

Please ask for commit rights, 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] avcodec/fitsdec: Prevent division by 0 with huge data_max

2019-07-18 Thread Reimar Döffinger
On 18.07.2019, at 12:54, Michael Niedermayer  wrote:

> On Thu, Jul 18, 2019 at 08:21:21AM +0200, Reimar Döffinger wrote:
>> 
>> 
>> On 16.07.2019, at 20:31, Michael Niedermayer  wrote:
>> 
>>> On Tue, Jul 16, 2019 at 08:34:14AM +0200, Reimar Döffinger wrote:
 On 16.07.2019, at 00:50, Michael Niedermayer  
 wrote:
 
> Fixes: division by 0
> Fixes: 
> 15657/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_FITS_fuzzer-5738154838982656
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
> libavcodec/fitsdec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/fitsdec.c b/libavcodec/fitsdec.c
> index 4f452422ef..fe941f199d 100644
> --- a/libavcodec/fitsdec.c
> +++ b/libavcodec/fitsdec.c
> @@ -174,7 +174,7 @@ static int fits_read_header(AVCodecContext *avctx, 
> const uint8_t **ptr, FITSHead
>   return AVERROR_INVALIDDATA;
>   }
>   av_log(avctx, AV_LOG_WARNING, "data min/max indicates a blank 
> image\n");
> -header->data_max ++;
> +header->data_max += fabs(header->data_max) / 1000 + 1;
 
 This is really non-obvious, both by itself, in where/how it causes the 
 division by 0 and that the error here isn't worse than the division by 0 
 for example, and why this constant was chosen.
>>> 
>>> division by 0 occured in:
>>> *dst++ = ((t - header.data_min) * ((1 << (sizeof(type) * 8)) - 1)) / 
>>> (header.data_max - header.data_min); \
>> 
>> I looked at the code, and now it makes even less sense to me.
>> First, why is that reported as an error at all?
>> Dividing by 0 is well defined for floating-point.
> 
> at least at the point where its stored in an integer it becomes painfull
> to the compiler to find a way to do it.

Hm, I am not quite following. The division results in inf or nan, and those get 
converted to integer the usual way (not sure how well that is defined in C, but 
it's not a division by 0 error).

> Iam not sure if inf is a problem (from a very quick look) that would get
> divided to 0 i guess nan would be an issue, i didnt think of this, i will 
> redo this and post a better patch

inf / inf = nan
From my point of view if 0.0 / 0.0 is considered an issue that seems like it 
should apply for inf as well.
When it comes to actually correct code, there might also be the question what 
to do in that case.
Could be considered "bad data/normalization range, just skip the whole scaling".
Or could define it like OpenGL (and other) normalization operations:
-inf becomes -1, inf becomes 1 all else 0. But that would need a special code 
case, and I guess it's not worth it.
___
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 v1] Bug #8027 - Wrong result for FFSIGN(0)

2019-07-18 Thread Hendrik Leppkes
On Thu, Jul 18, 2019 at 9:17 PM Ulf Zibis  wrote:
>
>
> Am 17.07.19 um 10:12 schrieb Hendrik Leppkes:
> > ..., the macro was defined the way it is, and changing it now has a
> > chance of breaking
> > random places that rely on the current behavior.
>
> OK, that would be risky.
>
> Maybe we could add another macro FFSGN. Then maintainers of individual
> code parts could check the usage over the time.
>
> #define FFSGN(a) ((a) > 0 ? 1 : (a) < 0 ? -1 : 0)
>

Unless something inside FFmpeg needs such a behavior, that is unlikely.

- Hendrik
___
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] Setup for extracting quantization parameters from encoded streams

2019-07-18 Thread Juan De León
Thanks for the feedback, appreciate it.
I added the AV_FRAME_DATA_QUANTIZATION_PARAMS type to AVFrameSideData and I 
removed the filter since it is not complete yet. 
I will send the filter and the modification to the encoders when those are done.

---
 libavutil/Makefile  |  2 +
 libavutil/frame.h   |  6 ++
 libavutil/quantization_params.c | 28 ++
 libavutil/quantization_params.h | 98 +
 4 files changed, 134 insertions(+)
 create mode 100644 libavutil/quantization_params.c
 create mode 100644 libavutil/quantization_params.h

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 8a7a44e4b5..be1a9c3a9c 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -60,6 +60,7 @@ HEADERS = adler32.h   
  \
   pixdesc.h \
   pixelutils.h  \
   pixfmt.h  \
+  quantization_params.h \
   random_seed.h \
   rc4.h \
   rational.h\
@@ -140,6 +141,7 @@ OBJS = adler32.o
\
parseutils.o \
pixdesc.o\
pixelutils.o \
+   quantization_params.o\
random_seed.o\
rational.o   \
reverse.o\
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 5d3231e7bb..c0afca325c 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -179,6 +179,12 @@ enum AVFrameSideDataType {
  * array element is implied by AVFrameSideData.size / 
AVRegionOfInterest.self_size.
  */
 AV_FRAME_DATA_REGIONS_OF_INTEREST,
+/**
+ * To extract quantization parameters from supported encoded streams.
+ * The data stored is AVQuantizationParamsArray type, described in
+ * libavuitls/quantization_params.h
+ */
+AV_FRAME_DATA_QUANTIZATION_PARAMS,
 };
 
 enum AVActiveFormatDescription {
diff --git a/libavutil/quantization_params.c b/libavutil/quantization_params.c
new file mode 100644
index 00..96ffd78dbb
--- /dev/null
+++ b/libavutil/quantization_params.c
@@ -0,0 +1,28 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/quantization_params.h"
+
+/**
+ * Strings for names of enums, used by Filter
+ */
+const char* const QP_NAMES_H264[] = {"qp"};
+const char* const QP_NAMES_VP9[] = {"qydc", "qyac", "quvdc", "quvac", "qiydc", 
"qiyac",
+  "qiuvdc", "qiuvac"};
+const char* const QP_NAMES_AV1[] = {"qydc", "qyac", "qudc", "quac", "qvdc", 
"qvac",
+  "qiydc", "qiyac", "qiudc", "qiuac", "qivdc", 
"qivac"};
\ No newline at end of file
diff --git a/libavutil/quantization_params.h b/libavutil/quantization_params.h
new file mode 100644
index 00..e986abe842
--- /dev/null
+++ b/libavutil/quantization_params.h
@@ -0,0 +1,98 @@
+/*
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, 

Re: [FFmpeg-devel] [PATCH] avcodec/tak_parser: don't return error values

2019-07-18 Thread Michael Niedermayer
On Wed, Jul 17, 2019 at 07:38:50PM -0300, James Almer wrote:
> The API does not allow it.
> 
> Also set poutbuf and poutbuf_size to NULL/0 on error to avoid leaving
> them uninitialized.
> 
> Signed-off-by: James Almer 
> ---
>  libavcodec/tak_parser.c | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/tak_parser.c b/libavcodec/tak_parser.c
> index 835a47bd52..7943d2ed3a 100644
> --- a/libavcodec/tak_parser.c
> +++ b/libavcodec/tak_parser.c
> @@ -49,7 +49,7 @@ static int tak_parse(AVCodecParserContext *s, 
> AVCodecContext *avctx,
>  if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) {
>  TAKStreamInfo ti;
>  if ((ret = init_get_bits8(&gb, buf, buf_size)) < 0)
> -return ret;
> +goto fail;
>  if (!ff_tak_decode_frame_header(avctx, &gb, &ti, 127))
>  s->duration = t->ti.last_frame_samples ? t->ti.last_frame_samples
> : t->ti.frame_samples;

this would occur if buf_size is too big ( >250mb )

[...]
> +fail:
> +
> +*poutbuf  = NULL;
> +*poutbuf_size = 0;
> +return buf_size + consumed;

and this would silently drop the data

i think thats not ideal, it would be better to pass through packets
which exceed the capabilities of the parser

Thanks

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH v2] avcodec/tak_parser: don't return error values

2019-07-18 Thread James Almer
The API does not allow it.

Also set poutbuf and poutbuf_size to NULL/0 on error to avoid leaving
them uninitialized.

Signed-off-by: James Almer 
---
 libavcodec/tak_parser.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/libavcodec/tak_parser.c b/libavcodec/tak_parser.c
index 835a47bd52..3604b35443 100644
--- a/libavcodec/tak_parser.c
+++ b/libavcodec/tak_parser.c
@@ -46,15 +46,16 @@ static int tak_parse(AVCodecParserContext *s, 
AVCodecContext *avctx,
 int needed   = buf_size ? TAK_MAX_FRAME_HEADER_BYTES : 8;
 int ret;
 
+*poutbuf  = buf;
+*poutbuf_size = buf_size;
+
 if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) {
 TAKStreamInfo ti;
 if ((ret = init_get_bits8(&gb, buf, buf_size)) < 0)
-return ret;
+return buf_size;
 if (!ff_tak_decode_frame_header(avctx, &gb, &ti, 127))
 s->duration = t->ti.last_frame_samples ? t->ti.last_frame_samples
: t->ti.frame_samples;
-*poutbuf  = buf;
-*poutbuf_size = buf_size;
 return buf_size;
 }
 
@@ -65,7 +66,7 @@ static int tak_parse(AVCodecParserContext *s, AVCodecContext 
*avctx,
 const uint8_t *tmp_buf = buf;
 
 if (ff_combine_frame(pc, END_NOT_FOUND, &tmp_buf, &tmp_buf_size) 
!= -1)
-return AVERROR(ENOMEM);
+goto fail;
 consumed += tmp_buf_size;
 buf  += tmp_buf_size;
 buf_size -= tmp_buf_size;
@@ -78,7 +79,7 @@ static int tak_parse(AVCodecParserContext *s, AVCodecContext 
*avctx,
 
 if ((ret = init_get_bits8(&gb, pc->buffer + t->index,
   pc->index - t->index)) < 0)
-return ret;
+goto fail;
 if (!ff_tak_decode_frame_header(avctx, &gb,
 pc->frame_start_found ? &ti : &t->ti, 127) &&
 !ff_tak_check_crc(pc->buffer + t->index,
@@ -103,9 +104,7 @@ found:
 
 if (consumed && !buf_size && next == END_NOT_FOUND ||
 ff_combine_frame(pc, next, &buf, &buf_size) < 0) {
-*poutbuf  = NULL;
-*poutbuf_size = 0;
-return buf_size + consumed;
+goto fail;
 }
 
 if (next != END_NOT_FOUND) {
@@ -116,6 +115,11 @@ found:
 *poutbuf  = buf;
 *poutbuf_size = buf_size;
 return next;
+
+fail:
+*poutbuf  = NULL;
+*poutbuf_size = 0;
+return buf_size + consumed;
 }
 
 AVCodecParser ff_tak_parser = {
-- 
2.22.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".

[FFmpeg-devel] [PATCH] fftools/ffprobe: process show_frame/show_packets last

2019-07-18 Thread Aman Gupta
From: Aman Gupta 

When using `ffprobe -show_format -show_streams -show_packets`,
it makes more sense to omit static data about the file format
and streams before the long list of packets instead of at the
end.

Signed-off-by: Aman Gupta 
---
 fftools/ffprobe.c | 38 --
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 5aaddb0308..c9c10b143d 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -3003,6 +3003,26 @@ static int probe_file(WriterContext *wctx, const char 
*filename)
 ifile.fmt_ctx->streams[i]->discard = AVDISCARD_ALL;
 }
 
+if (do_show_format) {
+ret = show_format(wctx, &ifile);
+CHECK_END;
+}
+
+if (do_show_streams) {
+ret = show_streams(wctx, &ifile);
+CHECK_END;
+}
+
+if (do_show_programs) {
+ret = show_programs(wctx, &ifile);
+CHECK_END;
+}
+
+if (do_show_chapters) {
+ret = show_chapters(wctx, &ifile);
+CHECK_END;
+}
+
 if (do_read_frames || do_read_packets) {
 if (do_show_frames && do_show_packets &&
 wctx->writer->flags & 
WRITER_FLAG_PUT_PACKETS_AND_FRAMES_IN_SAME_CHAPTER)
@@ -3019,24 +3039,6 @@ static int probe_file(WriterContext *wctx, const char 
*filename)
 CHECK_END;
 }
 
-if (do_show_programs) {
-ret = show_programs(wctx, &ifile);
-CHECK_END;
-}
-
-if (do_show_streams) {
-ret = show_streams(wctx, &ifile);
-CHECK_END;
-}
-if (do_show_chapters) {
-ret = show_chapters(wctx, &ifile);
-CHECK_END;
-}
-if (do_show_format) {
-ret = show_format(wctx, &ifile);
-CHECK_END;
-}
-
 end:
 if (ifile.fmt_ctx)
 close_input_file(&ifile);
-- 
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] dash: add descriptor which is useful to the scheme defined by ISO/IEC 23009-1:2014/Amd.2:2015.

2019-07-18 Thread Jeyapal, Karthick

On 7/19/19 8:38 AM, leozhang wrote:
> change history:
> 1. Use normal descriptor string instead of base64 encoded
> 2. Add example to muxers.texi
> 3. Change descriptor char * and allocate it dynamically
>
> Please review, thanks
Thanks for sending the revised patch. LGTM.
I will wait for 3 days and then push it, if there are no other objections.

Regards,
Karthick
>
> Signed-off-by: leozhang 
> ---
>  doc/muxers.texi   |  4 
>  libavformat/dashenc.c | 27 +++
>  2 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/doc/muxers.texi b/doc/muxers.texi
> index b109297..bc38cf6 100644
> --- a/doc/muxers.texi
> +++ b/doc/muxers.texi
> @@ -275,6 +275,10 @@ of the adaptation sets and a,b,c,d and e are the indices 
> of the mapped streams.
>  To map all video (or audio) streams to an AdaptationSet, "v" (or "a") can be 
> used as stream identifier instead of IDs.
>  
>  When no assignment is defined, this defaults to an AdaptationSet for each 
> stream.
> +
> +Optional syntax is "id=x,descriptor=descriptor_string,streams=a,b,c 
> id=y,streams=d,e" and so on, descriptor is useful to the scheme defined by 
> ISO/IEC 23009-1:2014/Amd.2:2015.
> +For example, -adaptation_sets "id=0,descriptor= schemeIdUri=\"urn:mpeg:dash:srd:2014\" value=\"0,0,0,1,1,2,2\"/>,streams=v".
> +Please note that descriptor string should be a self-closing xml tag.
>  @item timeout @var{timeout}
>  Set timeout for socket I/O operations. Applicable only for HTTP output.
>  @item index_correction @var{index_correction}
> diff --git a/libavformat/dashenc.c b/libavformat/dashenc.c
> index b25afb4..24f8d4d 100644
> --- a/libavformat/dashenc.c
> +++ b/libavformat/dashenc.c
> @@ -68,6 +68,7 @@ typedef struct Segment {
>  
>  typedef struct AdaptationSet {
>  char id[10];
> +char *descriptor;
>  enum AVMediaType media_type;
>  AVDictionary *metadata;
>  AVRational min_frame_rate, max_frame_rate;
> @@ -552,8 +553,10 @@ static void dash_free(AVFormatContext *s)
>  int i, j;
>  
>  if (c->as) {
> -for (i = 0; i < c->nb_as; i++)
> +for (i = 0; i < c->nb_as; i++) {
>  av_dict_free(&c->as[i].metadata);
> +av_freep(&c->as[i].descriptor);
> +}
>  av_freep(&c->as);
>  c->nb_as = 0;
>  }
> @@ -748,7 +751,8 @@ static int write_adaptation_set(AVFormatContext *s, 
> AVIOContext *out, int as_ind
>  role = av_dict_get(as->metadata, "role", NULL, 0);
>  if (role)
>  avio_printf(out, "\t\t\t schemeIdUri=\"urn:mpeg:dash:role:2011\" value=\"%s\"/>\n", role->value);
> -
> +if (as->descriptor)
> +avio_printf(out, "\t\t\t%s\n", as->descriptor);
>  for (i = 0; i < s->nb_streams; i++) {
>  OutputStream *os = &c->streams[i];
>  char bandwidth_str[64] = {'\0'};
> @@ -820,7 +824,7 @@ static int parse_adaptation_sets(AVFormatContext *s)
>  {
>  DASHContext *c = s->priv_data;
>  const char *p = c->adaptation_sets;
> -enum { new_set, parse_id, parsing_streams } state;
> +enum { new_set, parse_id, parsing_streams, parse_descriptor } state;
>  AdaptationSet *as;
>  int i, n, ret;
>  
> @@ -837,6 +841,9 @@ static int parse_adaptation_sets(AVFormatContext *s)
>  }
>  
>  // syntax id=0,streams=0,1,2 id=1,streams=3,4 and so on
> +// option id=0,descriptor=descriptor_str,streams=0,1,2 and so on
> +// descriptor is useful to the scheme defined by ISO/IEC 
> 23009-1:2014/Amd.2:2015
> +// descriptor_str should be a self-closing xml tag.
>  state = new_set;
>  while (*p) {
>  if (*p == ' ') {
> @@ -854,7 +861,19 @@ static int parse_adaptation_sets(AVFormatContext *s)
>  if (*p)
>  p++;
>  state = parse_id;
> -} else if (state == parse_id && av_strstart(p, "streams=", &p)) {
> +} else if (state == parse_id && av_strstart(p, "descriptor=", &p)) {
> +n = strcspn(p, ">") + 1; //followed by one comma, so plus 1
> +if (n < strlen(p)) {
> +as->descriptor = av_strndup(p, n);
> +} else {
> +av_log(s, AV_LOG_ERROR, "Parse error, descriptor string 
> should be a self-closing xml tag\n");
> +return AVERROR(EINVAL);
> +}
> +p += n;
> +if (*p)
> +p++;
> +state = parse_descriptor;
> +} else if ((state == parse_id || state == parse_descriptor) && 
> av_strstart(p, "streams=", &p)) { //descriptor is optional 
>  state = parsing_streams;
>  } else if (state == parsing_streams) {
>  AdaptationSet *as = &c->as[c->nb_as - 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".