Re: [FFmpeg-devel] [PATCH] mpeg12enc: Use all Closed Captions side data

2019-05-20 Thread Hendrik Leppkes
On Mon, May 20, 2019 at 9:44 AM Reimar Döffinger
 wrote:
>
>
>
> On 20.05.2019, at 09:23, Mathieu Duponchelle  wrote:
>
> > Thanks :)
> >
> > On 5/19/19 7:00 PM, Devin Heitmueller wrote:
> >> Hello Paul,
> >>
> >> On Fri, May 17, 2019 at 4:44 AM Paul B Mahol  wrote:
> >>> On 5/17/19, Mathieu Duponchelle  wrote:
>  There isn't one, as I said the added indentation is because of the new 
>  loop!
> >>> To get this committed to tree you need to comply to review requests.
> >> I think Mathieu's point is that the code indentation change was not
> >> cosmetic - it's because the code in question is now inside a for loop,
> >> and thus it needed to be indented another level.
> >>
> >> Are you suggesting he should make a patch which results in the
> >> indentation being wrong, and then submit a second patch which fixes
> >> the incorrect indentation introduced by the first patch?
>
> I think it should probably be up to the maintainer, but possibly yes.
> A lot of review still happens primarily by email, and if you re-indent code
> it becomes impossible to see what, if anything, you changed in that block.
> Enabling reviews of the patch via email means not re-indenting even if
> it becomes "wrong".
> Not everyone does reviews that way though, so some maintainers nowadays might 
> prefer it differently?

My main concern with such requests is that its added effort to even
make these patches, since many developers will just automatically
indent where appropriate (as it should be), or even have tooling to do
it for them.
Personally, I would have to use another editor to "un-indent" the
code, since my main editor automatically corrects this stuff for me in
newly written code. As such, I'll personally never comply with such
requests, because I think its silly, and it would be extraordinary
effort to even do it, first restoring original indent with a second
editor.

In so many cases the "cosmetic" second commit was also quite simply
forgotten, resulting in an ugly codebase. Nevermind the added noise in
the commit log.

I don't think its that much to ask for a reviewer to also use a tool
that can ignore whitespace. Maybe we can teach patchwork to offer a
button to view diffs without whitespace, if people don't want to use a
simple tool.

- 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] Populate MPEG2TS codec_tag using ff_codec_movvideo_tags/ff_codec_movaudio_tags (av4cc) rather than the PES stream_type. For MPEG2TS files containing h264, ffprobe currently

2019-05-16 Thread Hendrik Leppkes
On Thu, May 16, 2019 at 6:12 PM Damien Lévin
 wrote:
>
> Thanks Hendrik,
>
> The documentation from the AVCodecParameters codec_tag changed here (and
> not AVCodecContext which you are referring to) seems pretty explicit about
> being an AVI fourcc. So I'm not sure I understand why setting the PES
> stream_type (ISO/IEC 13818-1, table 2-34) would be more correct.
> "Additional information about the codec (corresponds to the AVI FOURCC)."
>
> If you look at the code from ffprobe.c (which also uses  AVCodecParameters)
> the assumption is to also have an AVI fourcc.
>
> /* print AVI/FourCC tag */
> print_str("codec_tag_string", av_fourcc2str(par->codec_tag));
> print_fmt("codec_tag", "0x%04"PRIx32, par->codec_tag);
>
> I understand that dumping the stream_type can be convenient but that
> doesn't seems like the correct behavior.
> Let me know what you think.
>

Dumping information in there thats not based on the container is
always going to be wrong. Why mov tags? Why not riff tags? Or some
other random format?
It should be container based or empty. And since the container just
has the pes stream type, thats what make sense. Always better to
convey something then nothing.

What are you even trying to fix?

- 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] "assert(a && b)" --> "assert(a); assert(b)" for more precise diagnostics, except for libformat

2019-05-15 Thread Hendrik Leppkes
On Wed, May 15, 2019 at 9:21 PM Adam Richter  wrote:
>
> On Tue, May 14, 2019 at 6:48 PM myp...@gmail.com  wrote:
> >
> > On Wed, May 15, 2019 at 7:01 AM Hendrik Leppkes  wrote:
> > >
> > > On Tue, May 14, 2019 at 11:25 PM Adam Richter  
> > > wrote:
> > > >
> > > > Consider, for example, if you agree that columnization makes this range 
> > > > check
> > > > more recognizable in a glance and makes it easier to spot what the 
> > > > bounds are
> > > > (the sixteen space indentation is taken from the code in which it 
> > > > appeared):
> > > >
> > > > av_assert0(par->bits_per_coded_sample >= 0 &&
> > > > par->bits_per_coded_sample <= 8);
> > > >
> > > > ...vs...
> > > >
> > > > av_assert0(par->bits_per_coded_sample >= 0);
> > > > av_assert0(par->bits_per_coded_sample <= 8);
> > > >
> > > > A possible counter-argument to this might be that, in a long sequence
> > > > of assertions, can be informative to group related assertions
> > > > together, which I think is true, but it is possible to get this other
> > > > means, such as by using blank lines to separate express such grouping.
> > > >
> > > > So, Mark, if you decide you are OK with my complete patches, I encourage
> > > > you to let me know.  Otherwise, if there are any of those changes which 
> > > > you
> > > > are OK with, I would like to just to to get those merged.  Finally, if 
> > > > you would
> > > > rather see none of those changes merged (one one to split the 
> > > > assertions in
> > > > libavformat and one was for everywhere else in ffmpeg), please let me 
> > > > know
> > > > about that too, in which case, if no one advocates for their
> > > > inclusion, I'll drop
> > > > my proposal to merge these changes.
> > > >
> > >
> > > Unfortunately I have to agree with Mark. asserst that check the same
> > > value or extremely closely related values should stay combined.
> > >
> > I agree this part
>
> I am trying to understand what problem you see with this.
>
> It occurs to me that you might be concerned about generating less
> efficient code for the common assert success case, in particular,
> in the example I showed for readability, potentially dereferencing
> "par" twice, but I made a test program (attached) and determined
> from reading the generated assembly that at least for gcc with
> optimization -O2 on x86_64, x86_32, aarch64 and arm32, as
> long as the abort function has "__attribute__ ((noreturn))", the
> compiler seems to be able to avoid repeating the memory fetch
> for the second assertion.  I also check this for clang -O2 on
> on x86_64.
>
> Of course, without the noreturn attribute on the abort function,
> the compiler realizes that the abort function could have written
> to memory, so it refetches the value in the split assertion case,
> which I think might have been your concern, but as long as
> the abort function is declared with an attribute noreturn, we
> should be OK for gcc.
>
> On the other hand, I'm not sure what compilers people use
> for other platforms such as Microsoft Windows and if you tell
> me that it is a known problem on a specific platform that is
> potentially relevant to ffmpeg, that would probably change my
> mind.
>
> Of course, it's not necessary for you change my mind or for
> you to invest further time in this discussion, as I imagine you
> and other participants have other things to do.  So, if I don't
> get a further explanation, I may still believe that you're wrong,
> but I'll respect your need to prioritize tasks other than continuing
> this discussion, and will not expect to see my proposed change
> merged unless the predominant opinion of others in this discussion
> changes to being in favor it, which, so far, I acknowledge, it is not.

You seem to be totally overthinking this. I'm not concerned about code
generation or anything like that, just the simple fact that I believe
that the checks are more logical and actually easier to understand if
they are logically grouped and combined. And I really don't see the
advantage in any of these changes, personally.

>
> > > >
> > > > Also after this, I may take a look at adding a branch hint to the 
> > > > av_assertN
> > > > macros if nobody objects.
> > > >
> &g

Re: [FFmpeg-devel] [PATCH] avcodec/qtrle: return last frame even if unchanged

2019-05-15 Thread Hendrik Leppkes
On Wed, May 15, 2019 at 8:41 PM Michael Niedermayer
 wrote:
>
> Fixes: Ticket7880
>
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/qtrle.c| 27 +--
>  tests/ref/fate/qtrle-8bit |  1 +
>  2 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/qtrle.c b/libavcodec/qtrle.c
> index 2c29547e5a..11226cb842 100644
> --- a/libavcodec/qtrle.c
> +++ b/libavcodec/qtrle.c
> @@ -45,6 +45,7 @@ typedef struct QtrleContext {
>
>  GetByteContext g;
>  uint32_t pal[256];
> +int need_flush;
>  } QtrleContext;
>
>  #define CHECK_PIXEL_PTR(n)   
>  \
> @@ -444,6 +445,12 @@ static av_cold int qtrle_decode_init(AVCodecContext 
> *avctx)
>  return 0;
>  }
>
> +static void qtrle_flush(AVCodecContext *avctx){
> +QtrleContext *s = avctx->priv_data;
> +
> +s->need_flush = 0;
> +}
> +
>  static int qtrle_decode_frame(AVCodecContext *avctx,
>void *data, int *got_frame,
>AVPacket *avpkt)
> @@ -454,11 +461,26 @@ static int qtrle_decode_frame(AVCodecContext *avctx,
>  int has_palette = 0;
>  int ret, size;
>
> +if (!avpkt->data) {
> +if (s->need_flush) {
> +s->need_flush = 0;
> +if ((ret = ff_reget_buffer(avctx, s->frame)) < 0)
> +return ret;
> +if ((ret = av_frame_ref(data, s->frame)) < 0)
> +return ret;
> +*got_frame = 1;
> +}
> +return 0;
> +}
> +
>  bytestream2_init(>g, avpkt->data, avpkt->size);
>
>  /* check if this frame is even supposed to change */
> -if (avpkt->size < 8)
> +if (avpkt->size < 8) {
> +s->need_flush = 1;
>  return avpkt->size;
> +}
> +s->need_flush = 0;
>
>  /* start after the chunk size */
>  size = bytestream2_get_be32(>g) & 0x3FFF;
> @@ -572,5 +594,6 @@ AVCodec ff_qtrle_decoder = {
>  .init   = qtrle_decode_init,
>  .close  = qtrle_decode_end,
>  .decode = qtrle_decode_frame,
> -.capabilities   = AV_CODEC_CAP_DR1,
> +.flush  = qtrle_flush,
> +.capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_DELAY,
>  };
> diff --git a/tests/ref/fate/qtrle-8bit b/tests/ref/fate/qtrle-8bit
> index 27bb8aad71..39a03b7b6c 100644
> --- a/tests/ref/fate/qtrle-8bit
> +++ b/tests/ref/fate/qtrle-8bit
> @@ -61,3 +61,4 @@
>  0,160,160,1,   921600, 0xcfd6ad2b
>  0,163,163,1,   921600, 0x3b372379
>  0,165,165,1,   921600, 0x36f245f5
> +0,166,166,1,   921600, 0x36f245f5

Does this actually work if there is a gap between the last frame and
the previously changed frame?
I'm not sure where it would save the proper timestamp.

- 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] Populate MPEG2TS codec_tag using ff_codec_movvideo_tags/ff_codec_movaudio_tags (av4cc) rather than the PES stream_type. For MPEG2TS files containing h264, ffprobe currently

2019-05-15 Thread Hendrik Leppkes
On Wed, May 15, 2019 at 6:15 PM Damien Levin
 wrote:
>
> ---
>  libavformat/mpegts.c  | 9 +++--
>  tests/ref/fate/concat-demuxer-simple2-lavf-ts | 4 ++--
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index 8a84e5cc19..79c0b78b1f 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -877,6 +877,13 @@ static void mpegts_find_stream_type(AVStream *st,
>  st->codecpar->codec_id   != types->codec_id) {
>  st->codecpar->codec_type = types->codec_type;
>  st->codecpar->codec_id   = types->codec_id;
> +if (types->codec_type == AVMEDIA_TYPE_VIDEO) {
> +st->codecpar->codec_tag =
> +ff_codec_get_tag(ff_codec_movvideo_tags, 
> types->codec_id);
> +} else if (types->codec_type == AVMEDIA_TYPE_AUDIO) {
> +st->codecpar->codec_tag =
> +ff_codec_get_tag(ff_codec_movaudio_tags, 
> types->codec_id);
> +}
>  st->internal->need_context_update = 1;
>  }
>  st->request_probe= 0;
> @@ -908,8 +915,6 @@ static int mpegts_set_stream_info(AVStream *st, 
> PESContext *pes,
> "stream=%d stream_type=%x pid=%x prog_reg_desc=%.4s\n",
> st->index, pes->stream_type, pes->pid, (char *)_reg_desc);
>
> -st->codecpar->codec_tag = pes->stream_type;
> -

The current behavior is correct, per the documentation of the field:
" A demuxer should set this to what is stored in the field used to
identify the codec."

Arbitrarily setting the field from a table without any relation to the
container is definitely not correct. If a user wants to know the tag
to be used in mov/mp4, they can determine that independently, but not
from the mpegts demuxer. This field is container-specific, and as such
exporting the pes stream type is actually the correct thing to do.

On a personal note, I actually use that value to detect certain
streams from Blu-rays and their secondary meaning, since every pes
stream type is clearly defined, the value can be used to eg. identify
if an audio stream is primary or secondary audio.

- 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] "assert(a && b)" --> "assert(a); assert(b)" for more precise diagnostics, except for libformat

2019-05-14 Thread Hendrik Leppkes
On Tue, May 14, 2019 at 11:25 PM Adam Richter  wrote:
>
> Consider, for example, if you agree that columnization makes this range check
> more recognizable in a glance and makes it easier to spot what the bounds are
> (the sixteen space indentation is taken from the code in which it appeared):
>
> av_assert0(par->bits_per_coded_sample >= 0 &&
> par->bits_per_coded_sample <= 8);
>
> ...vs...
>
> av_assert0(par->bits_per_coded_sample >= 0);
> av_assert0(par->bits_per_coded_sample <= 8);
>
> A possible counter-argument to this might be that, in a long sequence
> of assertions, can be informative to group related assertions
> together, which I think is true, but it is possible to get this other
> means, such as by using blank lines to separate express such grouping.
>
> So, Mark, if you decide you are OK with my complete patches, I encourage
> you to let me know.  Otherwise, if there are any of those changes which you
> are OK with, I would like to just to to get those merged.  Finally, if you 
> would
> rather see none of those changes merged (one one to split the assertions in
> libavformat and one was for everywhere else in ffmpeg), please let me know
> about that too, in which case, if no one advocates for their
> inclusion, I'll drop
> my proposal to merge these changes.
>

Unfortunately I have to agree with Mark. asserst that check the same
value or extremely closely related values should stay combined.

>
> Also after this, I may take a look at adding a branch hint to the av_assertN
> macros if nobody objects.
>

Please don't, we don't do branch hints anywhere, and this is a bad
place to start.
If an assert is truely performance sensitive (as in, it makes a
measurable difference), it should probably be moved from assert0 to
assert1, and as such is only enabled in testing builds.

- 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 2/2] Revert "lavf/utils: Allow url credentials to contain a slash."

2019-05-14 Thread Hendrik Leppkes
On Tue, May 14, 2019 at 10:34 PM Marton Balint  wrote:
>
>
>
> On Sun, 5 May 2019, Carl Eugen Hoyos wrote:
>
> > Am So., 5. Mai 2019 um 20:51 Uhr schrieb Marton Balint :
> >>
> >> This reverts commit dd06f022b07438d650c82255dff16908ba04244a.
> >>
> >> Fixes ticket #7871 and reopens ticket #7816.
> >
> > I'll send an alternative patch in a moment.
>
> Ping for this, I still think the revert is the best we can do here.
>

I agree. Slashes are flat out not allowed in that part of the URI and
will always result in ambigious parsing.

- 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] [DECISION] Project policy on closed source components

2019-05-13 Thread Hendrik Leppkes
On Mon, May 13, 2019 at 10:53 PM Carl Eugen Hoyos  wrote:
>
> > Release branches provide a guarantee of API, ABI and feature
> > stability.
>
> And we sadly did not always hold that guarantee=-(

Mistakes have been made. We shall strive to be better in the future,
and not use them as an excuse to push an agenda.

>
> > We shall not violate that for some petty feud.
>
> I wonder if this isn't exactly a case where it should be violated.
> Contrary to the cases where we - unfortunately - have
> violated it in the past.
>

No, we shall not.
___
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] [DECISION] Project policy on closed source components

2019-05-13 Thread Hendrik Leppkes
On Mon, May 13, 2019 at 10:37 PM Carl Eugen Hoyos  wrote:
>
> Am Mo., 13. Mai 2019 um 22:32 Uhr schrieb James Almer :
> >
> > On 5/13/2019 5:23 PM, Carl Eugen Hoyos wrote:
> > > Am Mo., 13. Mai 2019 um 22:18 Uhr schrieb James Almer :
> > >>
> > >> On 5/13/2019 5:13 PM, Carl Eugen Hoyos wrote:
> > >>> Am Mo., 13. Mai 2019 um 22:10 Uhr schrieb Marton Balint 
> > >>> :
>
> > > 1) Should libNDI support be removed from the ffmpeg codebase?
> > 
> >  Thanks for the votes, I counted 9 yes, 5 no, so majority is for 
> >  removal of
> >  libNDI, which is already done.
> > >>>
> > >>> The vote was not about the removal from libndi from release branches?
> > >>
> > >> No, features on releases are frozen, as changing them can result in
> > >> breakages for distros and package managers.
> > >
> > > We have broken distros and packages before, we would not break it
> > > in this case;-)
> >
> > We would. Distros and scripts would be broken, and it would not be pretty.
>
> Would you please be so kind as to explain (if possible in detail) how
> this would be possible in this specific case?
> I do not understand how removing a non-free dependency can break a
> binary distribution.
>

There are other people that use and build ffmpeg, and track release
branches. And they may as well be building only for themselves a
non-free binary.
Release branches provide a guarantee of API, ABI and feature
stability. We shall not violate that for some petty feud.

- 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 v2] libavutil: add an FFT & MDCT implementation

2019-05-13 Thread Hendrik Leppkes
On Mon, May 13, 2019 at 8:57 AM Reimar Döffinger
 wrote:
>
> On 13.05.2019, at 04:54, Pedro Arthur  wrote:
>
> > Em dom, 12 de mai de 2019 às 18:11, Hendrik Leppkes
> >  escreveu:
> >>
> >> On Sun, May 12, 2019 at 11:05 PM Carl Eugen Hoyos  
> >> wrote:
> >>>
> >>> But seriously: We are of course not allowed to remove copyright
> >>> statements, no matter if we consider them relevant or not.
> >>>
> >>
> >> Please provide a source for such claims.
> > The GPL license included in the header states that.
> >
> > GPL2 [1] - "keep intact all the notices that refer to this License"
> > GPL3 [2]  - "Requiring preservation of specified reasonable legal
> > notices or author attributions in that material"
> > MIT [3] (for completeness) - "The above copyright notice and this
> > permission notice shall be included in all copies or substantial
> > portions of the Software."
> >
> > [1] - https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
> > [2] - http://www.gnu.org/licenses/gpl.html
> > [3] - https://opensource.org/licenses/MI
>
> Besides the direct legals and ethics of it, please note that it also creates a
> serious (even if unlikely) risk to us and our users.
> In any kind of legal case, whether to defend against some "copyright troll" 
> or to
> enforce the license it might become necessary to find the author of the code.
> Not properly taking care of the license and copyright statement side exposes 
> our
> users to unnecessary risk and makes it harder to enforce our license.
> Even if nothing happens, the work companies have to do to show compliance
> (because their customers require it or to reduce their risk) becomes much 
> harder.
> Which is why the Linux kernel currently is working on cleaning up their 
> license
> header mess.
>

This argument is not really applicable because the headers do not
actually let you identify who holds all the copyright over the code in
the file. You need to go dig through years of Git history.

- 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 v2] libavutil: add an FFT & MDCT implementation

2019-05-12 Thread Hendrik Leppkes
On Sun, May 12, 2019 at 11:05 PM Carl Eugen Hoyos  wrote:
>
> But seriously: We are of course not allowed to remove copyright
> statements, no matter if we consider them relevant or not.
>

Please provide a source for such claims.

- 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 v2] libavutil: add an FFT & MDCT implementation

2019-05-12 Thread Hendrik Leppkes
On Sun, May 12, 2019 at 10:38 PM Carl Eugen Hoyos  wrote:
>
> Am So., 12. Mai 2019 um 22:37 Uhr schrieb Paul B Mahol :
> >
> > On 5/12/19, Carl Eugen Hoyos  wrote:
> > > Am Fr., 10. Mai 2019 um 17:15 Uhr schrieb Lynne :
> > >>
> > >> Patch updated again.
> > >> Made some more cleanups to the transforms, the tables and the main
> > >> context.
> > >> API changed again, now the init function populates the function pointer
> > >> for transform.
> > >> I decided that having a separate function would encourage bad usage (e.g.
> > >> calling
> > >> the function every time before doing a transform rather than storing the
> > >> pointer) when
> > >> we're trying to avoid the overhead of function calls.
> > >> Also adjusted file names to match the API.
> > >
> > > As said, this patch is not ok as long as the copyright statements are
> > > missing.
> > >
> >
> > You are mislead. No statements are necessary.
>
> Why do you think so?
>
> The commit message states that a part of the code is coming
> from a file with an extensive copyright statement: Why would
> it be ok to remove it?
>

The names at the top of the file are always going to be inaccurate,
and as such meaningless, because everyone that changed the file in a
meaningful way holds a copyright over those changes, and not everyone
is added to that list (typically, no-one beyond the original author is
present there).
Since the list is not complete, and as such does not allow you to
identify who actually holds all the copyright in such a file, its not
legally relevant.

Everyone of the authors still hold a copyright no matter if the name
is present there, or not.

- 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 1/2] Revert "avcodec/qtrle: Do not output duplicated frames on insufficient input"

2019-05-06 Thread Hendrik Leppkes
On Tue, May 7, 2019 at 1:39 AM Hendrik Leppkes  wrote:
>
> On Tue, May 7, 2019 at 12:34 AM Michael Niedermayer
>  wrote:
> >
> > On Sun, May 05, 2019 at 08:51:08PM +0200, Marton Balint wrote:
> > > This reverts commit a9dacdeea6168787a142209bd19fdd74aefc9dd6.
> > >
> > > I don't think it is a good idea to drop frames from CFR input just 
> > > because they
> > > are duplicated, that can cause issues for API users expecting CFR input. 
> > > Also
> > > it can cause issues at the end of file, if the last frame is a duplicated
> > > frame.
> > >
> > > Fixes ticket #7880.
> > >
> > > Signed-off-by: Marton Balint 
> > > ---
> > >  libavcodec/qtrle.c|  12 ++---
> > >  tests/ref/fate/qtrle-8bit | 109 
> > > ++
> > >  2 files changed, 115 insertions(+), 6 deletions(-)
> >
> > This change would make the decoder quite a bit slower. It also would make
> > encoding the output harder.
> > For example motion estimation would be run over unchanged frames even when
> > no cfr is wanted.
>
> This is simple:
> There is X input packets, any other decoder outputs X output frames.
> FFmpeg outputs Y output frames (where Y < X). How can this be correct
> decoding?
>
> If you want to lesten the burden of static frames, a filter to detect
> duplicates and make a stream VFR is what you should suggest, not
> making decoders act "unexpectedly".
>
> >
> > Also if one for consistency wants every decoder to not drop duplicated 
> > things
> > that will cause some major problems in other decoders.
> > Iam thinking of MPEG2 here, where the duplication is at a field level
> > perfectly progressive material would be turned into some mess with field
> > repetition in that case. Again undoing that in a subsequent stage would be
> > quite a bit harder and wastefull
> >
>
> There is quite a fundamental difference between CFR codecs where we
> end up not generating output for an input packet just because we feel
> like it, and the thought of somehow interpreting field repeat
> metadata. That just smells like deflection, lets not go there.
>

Also since you talk about making a "mess", this class of changes makes
quite a mess out of perfectly CFR material, to use your own
formulation. :)

- 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 1/2] Revert "avcodec/qtrle: Do not output duplicated frames on insufficient input"

2019-05-06 Thread Hendrik Leppkes
On Tue, May 7, 2019 at 12:34 AM Michael Niedermayer
 wrote:
>
> On Sun, May 05, 2019 at 08:51:08PM +0200, Marton Balint wrote:
> > This reverts commit a9dacdeea6168787a142209bd19fdd74aefc9dd6.
> >
> > I don't think it is a good idea to drop frames from CFR input just because 
> > they
> > are duplicated, that can cause issues for API users expecting CFR input. 
> > Also
> > it can cause issues at the end of file, if the last frame is a duplicated
> > frame.
> >
> > Fixes ticket #7880.
> >
> > Signed-off-by: Marton Balint 
> > ---
> >  libavcodec/qtrle.c|  12 ++---
> >  tests/ref/fate/qtrle-8bit | 109 
> > ++
> >  2 files changed, 115 insertions(+), 6 deletions(-)
>
> This change would make the decoder quite a bit slower. It also would make
> encoding the output harder.
> For example motion estimation would be run over unchanged frames even when
> no cfr is wanted.

This is simple:
There is X input packets, any other decoder outputs X output frames.
FFmpeg outputs Y output frames (where Y < X). How can this be correct
decoding?

If you want to lesten the burden of static frames, a filter to detect
duplicates and make a stream VFR is what you should suggest, not
making decoders act "unexpectedly".

>
> Also if one for consistency wants every decoder to not drop duplicated things
> that will cause some major problems in other decoders.
> Iam thinking of MPEG2 here, where the duplication is at a field level
> perfectly progressive material would be turned into some mess with field
> repetition in that case. Again undoing that in a subsequent stage would be
> quite a bit harder and wastefull
>

There is quite a fundamental difference between CFR codecs where we
end up not generating output for an input packet just because we feel
like it, and the thought of somehow interpreting field repeat
metadata. That just smells like deflection, lets not go there.

- 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]lavf/utils: Do not read "@" without ":" as user name separator

2019-05-05 Thread Hendrik Leppkes
On Sun, May 5, 2019 at 9:47 PM Carl Eugen Hoyos  wrote:
>
> Am So., 5. Mai 2019 um 21:18 Uhr schrieb Hendrik Leppkes 
> :
> >
> > On Sun, May 5, 2019 at 9:08 PM Carl Eugen Hoyos  wrote:
> > >
> > > Hi!
> > >
> > > Attached patch fixes ticket #7871 without re-introducing #7816.
> > >
> >
> > There is no patch here. However, please note that its perfectly valid
> > to have a username without a password (ie. an @ without a ":") - while
> > it is not valid to have a slash in there.
>
> > For the record, the original ticket 7816 was not even about a slash,
> > it was about URI encoding not being supported. As such, it was never
> > resolved in the first place (and closed unjustly), and cannot regress.
>
> There is no url encoding in ftp according to the rfc.
>

FTP is not being passed the URI at all, its entirely for the
convenience of the application. As such, that argument makes no sense.

Either way, this does not fix the underlying problem.

Take this perfectly valid URI: (it does not point to an actual file,
but that is irrelevant): http://ffmpeg.org:80/foo@bar
If you support slashes in the username or password, this points at a
host named "bar" and has login details - both a username and a
password. If you don't, then this points at ffmpeg.org, port 80, and
some valid path.

Only one of those interpretations is correct, and everyone else,
including the RFC, says it should be the second.

- 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 1/2] Revert "avcodec/qtrle: Do not output duplicated frames on insufficient input"

2019-05-05 Thread Hendrik Leppkes
On Sun, May 5, 2019 at 8:51 PM Marton Balint  wrote:
>
> This reverts commit a9dacdeea6168787a142209bd19fdd74aefc9dd6.
>
> I don't think it is a good idea to drop frames from CFR input just because 
> they
> are duplicated, that can cause issues for API users expecting CFR input. Also
> it can cause issues at the end of file, if the last frame is a duplicated
> frame.
>
> Fixes ticket #7880.
>

qtrle is not the only codec such a change was applied to, and I've in
the past objected to this class of changes without much support.
"Security" arguments were used to squat any counter-argument.

- 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]lavf/utils: Do not read "@" without ":" as user name separator

2019-05-05 Thread Hendrik Leppkes
On Sun, May 5, 2019 at 9:08 PM Carl Eugen Hoyos  wrote:
>
> Hi!
>
> Attached patch fixes ticket #7871 without re-introducing #7816.
>

There is no patch here. However, please note that its perfectly valid
to have a username without a password (ie. an @ without a ":") - while
it is not valid to have a slash in there.

For the record, the original ticket 7816 was not even about a slash,
it was about URI encoding not being supported. As such, it was never
resolved in the first place (and closed unjustly), and cannot regress.

- 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 v2] lavf/h264: Add support for raw h264 stream from Arecont camera, fixes ticket #5154

2019-05-04 Thread Hendrik Leppkes
On Sat, May 4, 2019 at 10:52 AM Shivam Goyal  wrote:
>
> The improved patch is for ticket #5154.
>
> Support for Raw h264 stream from Arecont Camera.
>
> Suggest any changes required.
>

The format-name based check in ff_raw_read_partial_packet is really
iffy. You can setup a specific read function in the AVInputFormat
struct, just can't use the macro to set it up then. Also keep the
probe and read function in the same file then.

- 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] avformat/mov: set AVFMT_SEEK_TO_PTS flag

2019-05-02 Thread Hendrik Leppkes
On Thu, May 2, 2019 at 7:42 PM Sasi Inguva
 wrote:
>
> Looks good to me. We were already doing PTS based seeking in MOV demuxer
> (although it's not perfect).
>

There are several problems with the MOV approach however. You cannot
look at AVStream->index_entries, take a timestamp of such an index,
and then ask the demuxer to seek to it. Because one is PTS and one is
DTS, you would end up somewhere else.
This mismatch is rather annoying if you want to use the index for
anything externally.

I know its a tad bit off-topic here, but this patch directly touches
this area, and I ran into this just quite recently.

Patch is good either way, it just documents what the demuxer does.

- 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] [DECISION] Project policy on closed source components

2019-04-29 Thread Hendrik Leppkes
On Mon, Apr 29, 2019 at 8:35 PM Marton Balint  wrote:
> We can't really change this for the better unless there is a somewhat
> "recognized" authority which has the power to make decisions, rules, and
> enforce them.
>
> I hoped that this can be the voting comitte.

The voting commitee is pretty much everyone, and as such way too many people.
I would've preferred a small "steering board" of maybe 5 people or
whatever you like to call it, but in the meeting this was decided in,
a vote choose otherwise... :)

- 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] avformat/aacdec: account for small frame sizes.

2019-04-29 Thread Hendrik Leppkes
On Mon, Apr 29, 2019 at 2:39 PM Menno de Gier  wrote:
>
> Some ADTS files have a first frame that's shorter than the 10 bytes that
> are being read while checking for ID3 tags.
>
> Fixes #7271
>

James already send another patch for this issue -
https://patchwork.ffmpeg.org/patch/12913/ - which seems to get by
without seeks, which seems better. Maybe test that one?

- 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] avformat/mux: skip parameter and pts checks for data muxer

2019-04-28 Thread Hendrik Leppkes
On Sun, Apr 28, 2019 at 11:57 AM Michael Niedermayer
 wrote:
>
> On Sat, Apr 27, 2019 at 10:01:53AM +0530, Gyan wrote:
> >
> >
> > On 27-04-2019 01:32 AM, Michael Niedermayer wrote:
> > >On Fri, Apr 26, 2019 at 06:38:37PM +0530, Gyan wrote:
> > >>  mux.c |9 -
> > >>  1 file changed, 8 insertions(+), 1 deletion(-)
> > >>d94a699f5dbc31a8ee8b7d1bdb33004d9cd95d46  
> > >>0001-avformat-mux-skip-parameter-and-pts-checks-for-data-.patch
> > >> From 5ec154870d9c559037598b41736bf5b216a756e0 Mon Sep 17 00:00:00 2001
> > >>From: Gyan Doshi 
> > >>Date: Fri, 26 Apr 2019 18:31:33 +0530
> > >>Subject: [PATCH] avformat/mux: skip parameter and pts checks for data 
> > >>muxer
> > >>
> > >>Allows to dump a malformed stream for external inspection or repair.
> > >>---
> > >>  libavformat/mux.c | 9 -
> > >>  1 file changed, 8 insertions(+), 1 deletion(-)
> > >>
> > >>diff --git a/libavformat/mux.c b/libavformat/mux.c
> > >>index 83fe1de78f..3699b572f2 100644
> > >>--- a/libavformat/mux.c
> > >>+++ b/libavformat/mux.c
> > >>@@ -290,6 +290,9 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > >>  goto fail;
> > >>  }
> > >>+if (!strcmp(of->name, "data"))
> > >>+goto bypass;
> > >>+
> > >>  for (i = 0; i < s->nb_streams; i++) {
> > >>  st  = s->streams[i];
> > >>  par = st->codecpar;
> > >>@@ -409,6 +412,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
> > >>  av_dict_set(>metadata, e->key, NULL, 0);
> > >>  }
> > >>+bypass:
> > >I think this skips a bit more than what would make sense
> > >(for example priv_data allocation but thats not the only odd thing)
> > >
> > >also iam not sure this is the ideal approuch.
> > >I mean "I want to dump inavlid data in a data muxer for debug"
> > >that seems a potentially valid request for other muxers too
> > >like the image muxer producing individual files per frame and
> > >so on
> > What would be the ideal approach?
>
> I guess the ideal approuch would be to allow the developer who needs
> this to override the check when the muxer in use can actually saftely
> mux it without the specific check.
> There are for example muxers which would not function properly with
> backward going dts or something like that
>

We already have a variety of flags in place, like if a muxer doesn't
care for timestamps at all, flag it AVFMT_NOTIMESTAMPS, and have code
that checks timestamps check for this flag. Checks based on muxer
names seem generally always wrong.

- 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 2/3] avfilter: add audio upsample filter

2019-04-25 Thread Hendrik Leppkes
On Thu, Apr 25, 2019 at 7:25 PM Nicolas George  wrote:
>
> Paul B Mahol (12019-04-25):
> > I did reacted. It plays well with negotiation.
>
> I missed that part, sorry. But it did not address my concern.
>
> > Can you be more specific what exactly you mean by "plays well with
> > sample rate negotiation"?
>
> If the user judges that this is the kind of sample rate change that is
> necessary, then when the negotiation detects a sample rate change is
> necessary, these filters are used.
>

The point of this kind of filtering is not that they are necessary to
perform filtering, because you can filter at any sample rate, but that
oversampling for certain filters, and downsampling after, improves
quality. As such, negotiation is not really equipped to handle it,
since its designed to deal with compatibility between filters, not
quality concerns, and you also wouldn't want to remove the ability to
filter at ordinary sample rates either.

- 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] Patchwork attribution

2019-04-19 Thread Hendrik Leppkes
On Fri, Apr 19, 2019 at 8:23 PM Lou Logan  wrote:
>
> On Thu, 18 Apr 2019 18:01:27 -0400
> "Lou Logan"  wrote:
> >
> > But we can certainly give it a try if you like. I'm not sure how patchwork 
> > will handle it.
>
> (Didn't realize my webmail client wasn't actually wrapping my text
> although it was doing it in the window at the expected lengths.)
>
> I just changed it from "munge" to "wrap" (only on ffmpeg-devel for
> now). If it doesn't work out then I'll just go back to the original
> setting of "accept" until if/when we find a better solution. We will
> lose some subscribers but the current patch author mangling is not
> worth the trouble in my opinion.

The first mail like that arrived, and handling of it  of course
depends on the mail client in question. But at least Patchwork doesn't
seem to like it much, it  quite simply still attributes it to the
"shared" mail.
Not sure if there  is some way to get Patchwork to improve in that regard.
___
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] tests: don't include TARGET_PATH in the sample path needlessly

2019-04-19 Thread Hendrik Leppkes
On Fri, Apr 5, 2019 at 12:06 AM James Almer  wrote:
>
> On 4/3/2019 7:17 PM, Hendrik Leppkes wrote:
> > The transcode() helper function will already prepend the TARGET_PATH to
> > the sample path, if its a relative path. This avoids an issue on
> > Windows, where the relative path check could fail.
> > ---
> >  tests/fate/ffmpeg.mak | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
> > index af7282f9ab..71ab2f1f63 100644
> > --- a/tests/fate/ffmpeg.mak
> > +++ b/tests/fate/ffmpeg.mak
> > @@ -95,7 +95,7 @@ fate-copy-trac2211-avi: CMD = transcode "h264 -r 14" 
> > $(TARGET_SAMPLES)/h264/bbc2
> >
> >  FATE_STREAMCOPY-$(call ENCDEC, APNG, APNG) += fate-copy-apng
> >  fate-copy-apng: fate-lavf-apng
> > -fate-copy-apng: CMD = transcode apng 
> > "$(TARGET_PATH)/tests/data/lavf/lavf.apng" apng "-c:v copy"
> > +fate-copy-apng: CMD = transcode apng tests/data/lavf/lavf.apng apng "-c:v 
> > copy"
>
> Should be ok, thanks.
>

Applied.

- 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] Patchwork attribution

2019-04-18 Thread Hendrik Leppkes
On Thu, Apr 18, 2019 at 9:04 PM Lou Logan  wrote:
>
> On Thu, Apr 18, 2019, at 4:51 AM, Hendrik Leppkes wrote:
> >
> > Whoever setup this ML sender rewriting thing should probably look into
> > options to also re-write the patch content and add a "From:" line in
> > there with the original name and email to avoid issues.
>
> I enabled this due DMARC as Timo correctly pointed out. This was due to the 
> seemingly increasing number of rejected/bounced messages from certain domains 
> resulting in the recipient being automatically unsubscribed from the list. I 
> changed this relatively recently (last month?). I was unaware of the 
> patchwork issue.
>
> This was implemented with the "dmarc_moderation_action" option within 
> Mailman. It is currently set to "Munge from" and was previously set to 
> "Accept". More info:
>
> https://wiki.list.org/DEV/DMARC
>

Did you try the "wrap" option? As I understand it, it would preserve
the original mail entirely, and rely on the mail client of the
recipient (ie. us) to properly unwrap the MIME container.
We've had various problems of authorship in patches getting mangled by
this, so if that could prevent it, that would be useful.

- 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] Patchwork attribution

2019-04-18 Thread Hendrik Leppkes
On Thu, Apr 18, 2019 at 3:31 PM Gyan  wrote:
>
>
>
> On 18-04-2019 06:13 PM, Hendrik Leppkes wrote:
> > On Thu, Apr 18, 2019 at 12:41 PM Gyan  wrote:
> >> Ok.  Then shouldn't it assign 'Sam John via ffmpeg-devel' as the author?
> >> It did do that for 'Oliver Collyer via ffmpeg-devel', the string wrongly
> >> substituted here.
> >> This looks like a parser failure in Patchwork.
> >>
> > All of those patches with re-written senders are send from the same
> > email, ffmpeg-devel@ffmpeg.org
> > Presumably patchwork only sends the email itself, not the name, and as
> > such assumes this is the same person.
> >
>
> In git master of patchwork, I see that if a different name is provided
> for the same email, this new name is updated in the Person db object and
> used.
>
> See
> https://github.com/getpatchwork/patchwork/blob/c4eca471a4a2cc3a2438e3ee6061df8988a251a6/patchwork/parser.py#L365
>
> Introduced in v2.1.1. Which version are we using?
>

This is still not tracked on a per-patch basis, but one global name
per email address, so it wouldn't really solve anything. It just
changes from first person to send a patch to latest person to do so -
altering all previously send patches as well.

 - 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]lavc/alac: Make a variable unsigned

2019-04-18 Thread Hendrik Leppkes
On Thu, Apr 18, 2019 at 2:54 PM Lauri Kasanen  wrote:
>
> On Thu, 18 Apr 2019 13:53:37 +0200
> Carl Eugen Hoyos  wrote:
>
> > Hi!
> >
> > Attached patch silences a warning that is shown with some gcc versions.
>
> It pokes my style sense to have different things in the sizeof() and
> the var. How about uint32_t in both?
>

Those two things are entirely unrelated types, though.

- 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] Patchwork attribution

2019-04-18 Thread Hendrik Leppkes
On Thu, Apr 18, 2019 at 12:41 PM Gyan  wrote:
>
>
>
> On 18-04-2019 03:25 PM, Timo Rothenpieler wrote:
> > On 18/04/2019 09:49, Gyan wrote:
> >>
> >> Patchwork can incorrectly assign ownership. See
> >> https://patchwork.ffmpeg.org/patch/12680/
> >>
> >> The author is Sam John as identified by Message ID as well as the
> >> From field in the headers, yet Patchwork attributes this patch to
> >> "Oliver Collyer via ffmpeg-devel", a name which appears nowhere in
> >> the headers or in the submitted patch. The downloaded mbox patch
> >> shows the same wrong attribution. Does anyone know what's going on?
> >>
> >> Gyan
> >> ___
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> >
> > This happens due to Mailman having to rewrite peoples E-Mail addresses
> > if they have strong DKIM/DMARC configured on their sending Domain.
> > If that is the case, only the server of the actual Domain who owns the
> > right private key can send E-Mails under the name of that Domain.
> >
> > So for mailing lists to be able to send on the messages, they are
> > forced to change the sender address.
> > In the case of this list:
> > Sam John 
> > gets translated to.
> > Sam John via ffmpeg-devel 
> >
>
> Ok.  Then shouldn't it assign 'Sam John via ffmpeg-devel' as the author?
> It did do that for 'Oliver Collyer via ffmpeg-devel', the string wrongly
> substituted here.
> This looks like a parser failure in Patchwork.
>

All of those patches with re-written senders are send from the same
email, ffmpeg-devel@ffmpeg.org
Presumably patchwork only sends the email itself, not the name, and as
such assumes this is the same person.

Whoever setup this ML sender rewriting thing should probably look into
options to also re-write the patch content and add a "From:" line in
there with the original name and email to avoid issues.

- 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 V1 2/2] lavf: bump version/add Changelog entry when cleanup applehttp

2019-04-16 Thread Hendrik Leppkes
On Tue, Apr 16, 2019 at 10:58 AM Carl Eugen Hoyos  wrote:
>
> 2019-04-16 10:52 GMT+02:00, myp...@gmail.com :
> > On Tue, Apr 16, 2019 at 4:42 PM Hendrik Leppkes  wrote:
> >>
> >> On Tue, Apr 16, 2019 at 7:57 AM Jun Zhao  wrote:
> >> >
> >> > From: Jun Zhao 
> >> >
> >> > commit abfeba9 "lavf/hls: Cleanup the applehttp" missed
> >> > the version bump and Changelog entry.
> >> >
> >> > Signed-off-by: Jun Zhao 
> >> > ---
> >> >  Changelog |1 +
> >> >  libavformat/version.h |2 +-
> >> >  2 files changed, 2 insertions(+), 1 deletions(-)
> >> >
> >> > diff --git a/Changelog b/Changelog
> >> > index 5b2b1e5..2930471 100644
> >> > --- a/Changelog
> >> > +++ b/Changelog
> >> > @@ -24,6 +24,7 @@ version :
> >> >  - KUX demuxer
> >> >  - AV1 frame split bitstream filter
> >> >  - lscr decoder
> >> > +- cleanup applehttp in hls demuxer
> >> >
> >>
> >> If you read the other changelog entries, does this seem like matching
> >> the pattern in there? :)
> >> ChangeLog is for end-users, for changes that developers should care
> >> about, there is APIchanges.
>
> Without a version bump?
>

Well thats no reason to put it into ChangeLog regardless, because it
just doesn't belong in there.
This is runtime API, not compile-time API anyway, so someone needing
this string for some reason and wanting to support all versions will
have to check both strings regardless of version information,

- 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 V1 2/2] lavf: bump version/add Changelog entry when cleanup applehttp

2019-04-16 Thread Hendrik Leppkes
On Tue, Apr 16, 2019 at 7:57 AM Jun Zhao  wrote:
>
> From: Jun Zhao 
>
> commit abfeba9 "lavf/hls: Cleanup the applehttp" missed
> the version bump and Changelog entry.
>
> Signed-off-by: Jun Zhao 
> ---
>  Changelog |1 +
>  libavformat/version.h |2 +-
>  2 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/Changelog b/Changelog
> index 5b2b1e5..2930471 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -24,6 +24,7 @@ version :
>  - KUX demuxer
>  - AV1 frame split bitstream filter
>  - lscr decoder
> +- cleanup applehttp in hls demuxer
>

If you read the other changelog entries, does this seem like matching
the pattern in there? :)
ChangeLog is for end-users, for changes that developers should care
about, there is APIchanges.

- 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 v2] lavf/rtsp.c: Fix stimeout option not applied on http tunnel

2019-04-15 Thread Hendrik Leppkes
On Mon, Apr 15, 2019 at 12:46 PM Carl Eugen Hoyos  wrote:
>
> 2019-04-15 9:42 GMT+02:00, Liu Steven :
> >
> >
> >> 在 2019年4月11日,下午12:03,Liu Steven  写道:
> >>
> >>
> >>
> >>> 在 2019年4月11日,上午11:55,Jun Li  写道:
> >>> ...
> >>>
> >>> Ping.
> >> LGTM
> >
> > Pushed
>
> Who wrote the patch that you pushed?
>

Also please carefully review anything you want to push. Needing 3
tries for one commit is just ridicoulous. Reverts still stay in the
history for ever.

- 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] avcodec: add drop_changed_frames

2019-04-15 Thread Hendrik Leppkes
On Mon, Apr 15, 2019 at 8:17 AM Gyan  wrote:
>
>
>
> On 15-04-2019 12:17 AM, James Almer wrote:
> > On 4/14/2019 3:29 PM, Hendrik Leppkes wrote:
> >> On Sun, Apr 14, 2019 at 6:50 PM Gyan  wrote:
> >>> Implemented this patch
> >>> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html
> >>>
> >>>
> >>>in libavcodec as suggested by Michael
> >>>
> >> This sure adds a lot of additional fields to the main struct for a
> >> rather specialized feature, that I personally rather see in the hands
> >> of the user of avcodec, not avcodec itself.
> >>
> >> In any case, can't we do this without any new public fields at all?
> >> Put the initial_* state fields into an internal struct (ie.
> >> AVCodecInternal), and expose enabling this through
> >> AVCodecContext->flags or flags2?
> >> That would make me feel much less dirty looking at this patch, personally.
> > +1
> >
> > There has been work to turn public AVCodecContext fields into internal
> > codec options recently as they were too specialized. This commit adds
> > half a dozen of such fields in one go, so it just feels wrong.
>
> Since, for this proposed use, the initial frame information has to be
> recorded, I thought to make them public, so any avcodec user could check
> them for other purposes. Right now, their population is contingent on
> the changed option being set, but that can be taken care of. But no firm
> preference here.
>
> I have a soft preference to keep the first field distinct in avctx, as
> I've seen CLI users to be sloppy both with option placement and setting
> multiple flags correctly for both the same or different target streams.
>

CLI usability problems should be resolved in the CLI tools, not in the
avcodec API, imho.

- 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] avcodec: add drop_changed_frames

2019-04-14 Thread Hendrik Leppkes
On Sun, Apr 14, 2019 at 6:50 PM Gyan  wrote:
>
> Implemented this patch
> http://www.ffmpeg.org/pipermail/ffmpeg-devel/2019-March/241733.html
>
>
>   in libavcodec as suggested by Michael
>

This sure adds a lot of additional fields to the main struct for a
rather specialized feature, that I personally rather see in the hands
of the user of avcodec, not avcodec itself.

In any case, can't we do this without any new public fields at all?
Put the initial_* state fields into an internal struct (ie.
AVCodecInternal), and expose enabling this through
AVCodecContext->flags or flags2?
That would make me feel much less dirty looking at this patch, personally.

- 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] libavcodec/libdav1d: add libdav1d_get_format method to call ff_get_format

2019-04-12 Thread Hendrik Leppkes
On Sat, Apr 13, 2019 at 1:17 AM Lukas Rusak  wrote:
>
> On Thu, 2019-04-11 at 21:03 +0200, Hendrik Leppkes wrote:
> > On Thu, Apr 11, 2019 at 7:47 PM Peter F 
> > wrote:
> > > Hi,
> > >
> > > Am Do., 11. Apr. 2019 um 00:50 Uhr schrieb Hendrik Leppkes
> > > :
> > > > On Thu, Apr 11, 2019 at 12:39 AM Lukas Rusak 
> > > > wrote:
> > > > > This will allow applications to properly init the decoder in
> > > > > cases where a hardware decoder is tried first and and software
> > > > > decoder is tried after by calling the get_format callback.
> > > > >
> > > > > Even though there is no hardware pixel formats available
> > > > > we still need to return the software pixel format.
> > > > >
> > > > > Tested with Kodi by checking if multithreaded software
> > > > > decoding is properly activated.
> > > > > ---
> > > > >  libavcodec/libdav1d.c | 12 +++-
> > > > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > > >
> > > >
> > > > This doesn't make sense to m e. get_format isn't called on a wide
> > > > variety of decoders, and is only useful when there is a hardware
> > > > format of any kind.
> > > > Please elaborate what exactly this is trying to achieve.
> > >
> > > Can you elaborate on how to use ffmpeg's API properly as a client
> > > to
> > > decide if a decoder is a SW decoder and therefore howto properly
> > > setup
> > > (multi-)threading, especially it cannot be changed once
> > > initialized?
> > >
> >
> > I think you are approaching this from the wrong side. Even if the
> > decoder would support hardware, generally hardware support is
> > limited.
> > So if someone plays a 10-bit  H264 file, which no consumer hardware
> > supports, or even worse, a very high resolution file which is beyond
> > hardware limits, do you want to run single-threaded and slow? I hope
> > not.
> >
> > The way I solve that is to just close the decoder and re-open it if
> > needed, so I can change such settings. You also fare much  better if
> > you accept that you might  need to hard-code which codecs support
> > hardware at all. Considering thats one new one every 4 years or so,
> > it'll probably be fine.
> >
> > - Hendrik
>
> So why don't the software only formats follow the api and also call
> ff_get_format like is done here? That would stop from applications
> having to hardcode the hardware decoding formats.
>

Because the callback is useless if there is only one format.
Its meant to negotiate hardware formats, not inform the user that
there is no hardware acceleration. There is also dozens if not
hundreds of codecs, changing them all to add the callback would be a
lot of work for apparently no benefit.

If you just want to know if a decoder supports hardware acceleration
of any kind, then check AVCodec.hw_configs, it contains plenty
information to make such decisions without hardcoding anything.

- 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] libavcodec/libdav1d: add libdav1d_get_format method to call ff_get_format

2019-04-11 Thread Hendrik Leppkes
On Thu, Apr 11, 2019 at 7:47 PM Peter F  wrote:
>
> Hi,
>
> Am Do., 11. Apr. 2019 um 00:50 Uhr schrieb Hendrik Leppkes
> :
> >
> > On Thu, Apr 11, 2019 at 12:39 AM Lukas Rusak  wrote:
> > >
> > > This will allow applications to properly init the decoder in
> > > cases where a hardware decoder is tried first and and software
> > > decoder is tried after by calling the get_format callback.
> > >
> > > Even though there is no hardware pixel formats available
> > > we still need to return the software pixel format.
> > >
> > > Tested with Kodi by checking if multithreaded software
> > > decoding is properly activated.
> > > ---
> > >  libavcodec/libdav1d.c | 12 +++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> >
> > This doesn't make sense to m e. get_format isn't called on a wide
> > variety of decoders, and is only useful when there is a hardware
> > format of any kind.
> > Please elaborate what exactly this is trying to achieve.
>
> Can you elaborate on how to use ffmpeg's API properly as a client to
> decide if a decoder is a SW decoder and therefore howto properly setup
> (multi-)threading, especially it cannot be changed once initialized?
>

I think you are approaching this from the wrong side. Even if the
decoder would support hardware, generally hardware support is limited.
So if someone plays a 10-bit  H264 file, which no consumer hardware
supports, or even worse, a very high resolution file which is beyond
hardware limits, do you want to run single-threaded and slow? I hope
not.

The way I solve that is to just close the decoder and re-open it if
needed, so I can change such settings. You also fare much  better if
you accept that you might  need to hard-code which codecs support
hardware at all. Considering thats one new one every 4 years or so,
it'll probably be fine.

- 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, v2] lavu/hwcontext_qsv: Fix the realign check for hwupload

2019-04-11 Thread Hendrik Leppkes
On Thu, Apr 11, 2019 at 4:51 AM Li, Zhong  wrote:
>
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> > Of Linjie Fu
> > Sent: Wednesday, April 10, 2019 7:56 PM
> > To: ffmpeg-devel@ffmpeg.org
> > Cc: Fu, Linjie 
> > Subject: [FFmpeg-devel] [PATCH, v2] lavu/hwcontext_qsv: Fix the realign
> > check for hwupload
> >
> > Fix the aligned check in hwupload, input surface should be 16 aligned too.
> >
> > Fix #7830.
> >
> > Signed-off-by: Linjie Fu 
> > ---
> >
> >  libavutil/hwcontext_qsv.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavutil/hwcontext_qsv.c b/libavutil/hwcontext_qsv.c index
> > b6d8bfe2bf..8b000fe636 100644
> > --- a/libavutil/hwcontext_qsv.c
> > +++ b/libavutil/hwcontext_qsv.c
> > @@ -892,7 +892,8 @@ static int
> > qsv_transfer_data_to(AVHWFramesContext *ctx, AVFrame *dst,
> >  return ret;
> >
> >
> > -if (src->height & 16 || src->linesize[0] & 16) {
> > +if (src->height & 15 || src->width & 15 ||
> > +src->linesize[0] & 15) {
>
> Should be better to use FFALIGN()

Since you are checking alignment here, the manual checks should be
fine. FFALIGN is for creating aligned values. You could use something
like "if (src->height != FFALIGN(src->height, 16))", but that doesn't
really seem clearer to me.

>
> Another question is it really necessary to check width alignment if we 
> already checked linesize to fix this issue?
> (I guess it it not necessary, and if it is needed, many other places probably 
> needed to be changed too.)

I agree, linesize should be enough, and width alignment probably not needed.

- 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] libavcodec/libdav1d: add libdav1d_get_format method to call ff_get_format

2019-04-10 Thread Hendrik Leppkes
On Thu, Apr 11, 2019 at 12:39 AM Lukas Rusak  wrote:
>
> This will allow applications to properly init the decoder in
> cases where a hardware decoder is tried first and and software
> decoder is tried after by calling the get_format callback.
>
> Even though there is no hardware pixel formats available
> we still need to return the software pixel format.
>
> Tested with Kodi by checking if multithreaded software
> decoding is properly activated.
> ---
>  libavcodec/libdav1d.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>

This doesn't make sense to m e. get_format isn't called on a wide
variety of decoders, and is only useful when there is a hardware
format of any kind.
Please elaborate what exactly this is trying to achieve.

- 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] [RFC PATCH] avformat/utils: always seek back after avformat_find_stream_info()

2019-04-10 Thread Hendrik Leppkes
On Wed, Apr 10, 2019 at 9:55 AM Aman Gupta  wrote:
>
> On Tue, Apr 9, 2019 at 9:49 PM Hendrik Leppkes  wrote:
>
> > On Wed, Apr 10, 2019 at 2:21 AM Aman Gupta  wrote:
> > >
> > > From: Aman Gupta 
> > >
> > > Previously, the initial seek position was recorded into
> > > old_offset at the beginning of avformat_find_stream_info(),
> > > and passed into estimate_timings(). In the case of mpegts
> > > with a known filesize, it was further passed into
> > > estimate_timings_from_pts() which called avio_seek(SEEK_SET)
> > > after doing its timing related seeks. (Interestingly, this
> > > seeked all the way back to the initial position before
> > > the probe, rather than only back before the timing
> > > related seeks to the end of the file).
> > >
> > > With this commit, we pull the avio_seek() out of the mpegts
> > > specific code-path and unconditionally seek all streams back
> > > to their original position after probing is complete.
> > >
> > > This effectively prevents data that is consumed during
> > > avformat_find_stream_info() from being discarded, so packets
> > > contained at the beginning of a file are still passed back
> > > to the user for playback.
> > >
> >
> > I don't think I ever had a case where data was apparently lost from
> > the beginning of the stream. Can you give examples of what this fixes?
>
>
> Perhaps I'm missing something.
>
> If I see the debug log message "After avformat_find_stream_info() pos:
> 100 bytes  seeks:0", are the packets from the first megabyte of the
> file still returned from API? Maybe they are being cached somewhere and I
> did not notice.
>

As far as I know, the packets read during stream info probing are
being cached in a buffer and returned to the caller on av_read_frame.

- 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 2/2] avformat/mxfenc: support XAVC long gop

2019-04-09 Thread Hendrik Leppkes
On Wed, Apr 10, 2019 at 3:28 AM James Almer  wrote:
> >
> >> Two thirds of SPS is not hard to implement, so i really don't understand
> >> why you're so adamantly against it.
> >
> > I’m adamant about re-using code between libavcodec and libavformat.
> > Re-using code is _good_
>
> So lets do what i suggested in a previous email if re-implementing sps
> in libavformat is not ok in your opinion: Add an
> avpriv_h264_decode_seq_parameter_set() function that internally calls
> ff_h264_decode_seq_parameter_set(), including proper AVBufferRef
> cleaning at the end with a call to ff_h264_ps_uninit(), and that either
> returns the six values using pointers parameters, or takes a pointer to
> a new, small struct as parameter which will be allocated at runtime
> (thus avoiding storing it on stack within libavformat, and tying to the
> ABI) which contains at least those six values in question.
> The reason i suggest a new small struct is to avoid using H264ParamSets
> for this, which would imply direct access to ps->sps fields within
> libavformat, and thus locking everything in its current offset.
>
> See how avpriv_ac3_parse_header() and av_ac3_parse_header() wrap
> ff_ac3_parse_header() and each do either of the two options above.

As mentioned in another thread on this topic already, I agree with
this. If you insist on re-use so strongly, then lets not cement more
internal structs into the ABI, especially some that we worked on to
get out of the ABI recently in the first place, and define a clean
wrapper API that takes simple byte pointers as input and gives you a
simplified public struct back.

- 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] [RFC PATCH] avformat/utils: always seek back after avformat_find_stream_info()

2019-04-09 Thread Hendrik Leppkes
On Wed, Apr 10, 2019 at 2:21 AM Aman Gupta  wrote:
>
> From: Aman Gupta 
>
> Previously, the initial seek position was recorded into
> old_offset at the beginning of avformat_find_stream_info(),
> and passed into estimate_timings(). In the case of mpegts
> with a known filesize, it was further passed into
> estimate_timings_from_pts() which called avio_seek(SEEK_SET)
> after doing its timing related seeks. (Interestingly, this
> seeked all the way back to the initial position before
> the probe, rather than only back before the timing
> related seeks to the end of the file).
>
> With this commit, we pull the avio_seek() out of the mpegts
> specific code-path and unconditionally seek all streams back
> to their original position after probing is complete.
>
> This effectively prevents data that is consumed during
> avformat_find_stream_info() from being discarded, so packets
> contained at the beginning of a file are still passed back
> to the user for playback.
>

I don't think I ever had a case where data was apparently lost from
the beginning of the stream. Can you give examples of what this fixes?

- 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 1/2] avcodec/h264_parse: change prefix to avpriv for usage in avformat mxf muxer

2019-04-09 Thread Hendrik Leppkes
On Wed, Apr 10, 2019 at 12:44 AM Mark Thompson  wrote:
>
> On 09/04/2019 23:14, Baptiste Coudurier wrote:
> > ---
> >  libavcodec/h264_parse.c  | 2 +-
> >  libavcodec/h264_parser.c | 2 +-
> >  libavcodec/h264_ps.c | 4 ++--
> >  libavcodec/h264_ps.h | 4 ++--
> >  libavcodec/h264dec.c | 6 +++---
> >  5 files changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c
> > index 17bfa780ce..980b1e189d 100644
> > --- a/libavcodec/h264_ps.c
> > +++ b/libavcodec/h264_ps.c
> > @@ -330,8 +330,8 @@ void ff_h264_ps_uninit(H264ParamSets *ps)
> >  ps->sps = NULL;
> >  }
> >
> > -int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext 
> > *avctx,
> > - H264ParamSets *ps, int 
> > ignore_truncation)
> > +int avpriv_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext 
> > *avctx,
> > + H264ParamSets *ps, int 
> > ignore_truncation)
> >  {
> >  AVBufferRef *sps_buf;
> >  int profile_idc, level_idc, constraint_set_flags = 0;
> > diff --git a/libavcodec/h264_ps.h b/libavcodec/h264_ps.h
> > index e967b9cbcf..d422ce122e 100644
> > --- a/libavcodec/h264_ps.h
> > +++ b/libavcodec/h264_ps.h
> > @@ -149,8 +149,8 @@ typedef struct H264ParamSets {
> >  /**
> >   * Decode SPS
> >   */
> > -int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext 
> > *avctx,
> > - H264ParamSets *ps, int 
> > ignore_truncation);
> > +int avpriv_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext 
> > *avctx,
> > + H264ParamSets *ps, int 
> > ignore_truncation);
>
> Making the H.264 decoder's internal SPS and PPS structures fixed API really 
> doesn't feel like a good idea to me, but I admit I don't have a better answer 
> to the problem you're facing.  Copying everything is also pretty terrible.  
> Adding a new API to parse the headers and return the information you want 
> might be nicer considering just this change, but extensibility for the 
> inevitable subsequent patch which wants some extra piece of information in 
> future would be a big pain.
>
> If you're going to go with this, please add namespace prefixes to the 
> structures it affects ("SPS" and "PPS", since you're now including them in 
> code which isn't explicitly H.264), and also add big warnings to the header 
> indicating that the structures are now fixed and can't be modified at all 
> except on major bumps.  (Though maybe wait for other comments before actually 
> making any changes.)
>

I agree that I really don't like exporting these functions. For
example, this function takes an AVCodecContext. Its reasonable to
assume when touching this function right now that its a valid
AVCodecContext with a valid H264Context inside of it. Apparently right
now its only used for logging, so some crazy cast appears to work, but
in the future?
Internal decoder functionality has no place being exported, it puts
too many limits in place for future changes.

Either the API needs to be wrapped in a proper external interface,
leaving our internal interface open to changes, or a simplified
version should be implemented in avformat, like we already did for a
bunch of other video formats as it was required. The SPS isn't that
complex tbh, so maybe implementing a simplified parser for mxf might
be the best option.

- 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 2/2] avformat/mxfenc: support XAVC long gop

2019-04-09 Thread Hendrik Leppkes
On Wed, Apr 10, 2019 at 12:21 AM Baptiste Coudurier
 wrote:
> +return 0;
> +}
> +init_get_bits(, tmp, tmp_size*8);
> +ret = avpriv_h264_decode_seq_parameter_set(, 
> (AVCodecContext*)s, , 0);

The AVCodecContext cast looks like a recipe for disaster. The function
is internal to the H264 decoder, so if it at some point decides to
actually access the AVCodecContext it receives, or even worse, the
priv_data element in it, expecting a H264Context, then everything goes
to hell.
Just another sign why exporting functions internal from a decoder is
just an overall bad idea...

- 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] [DECISION] Include more developers in the voting committee [#4]

2019-04-08 Thread Hendrik Leppkes
On Sat, Apr 6, 2019 at 6:42 PM Balint Marton  wrote:
>
> Hi All
>
> Here is a call for the people in the voting committee [1] on the decision
> of extending it.
>
> Using the same guidelines as in the second extension [2], the following
> candidates were found:
>
> git log libav/master..master --no-merges  --since=2018-03-30T00:00:00Z 
> --until 2019-03-30T00:00:00Z --pretty=fuller | grep '^Commit:' | sed 
> 's/<.*//' |sort | uniq -c | sort -nr
>
>  662 Commit: Michael Niedermayer
>  528 Commit: Paul B Mahol
>  227 Commit: James Almer
>  194 Commit: Carl Eugen Hoyos
>  183 Commit: Mark Thompson
>  141 Commit: Marton Balint
>   99 Commit: Jun Zhao
>   84 Commit: Steven Liu
>   73 Commit: Martin Vignali
>   64 Commit: Gyan Doshi
>   55 Commit: Aman Gupta
>   42 Commit: Timo Rothenpieler
>   35 Commit: Zhong Li
>   31 Commit: Karthick Jeyapal
>   31 Commit: Karthick J
>   23 Commit: Rostislav Pehlivanov
>   22 Commit: Peter Ross
>   22 Commit: Clément Bœsch
>   21 Commit: Lou Logan
>   21 Commit: Jan Ekström
>
> Some of these developers are already in the voting committee, the new ones
> would be:
>
> Aman Gupta
> Gyan Doshi
> Jan Ekström
> Jun Zhao
> Karthick Jeyapal
> Mark Thompson
> Martin Vignali
> Peter Ross
> Steven Liu
> Timo Rothenpieler
> Zhong Li
>

Yes.
___
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 RFC v2 2/3] libavcodec: Add thumbnail output to vaapi_h264 decoder

2019-04-08 Thread Hendrik Leppkes
On Mon, Apr 8, 2019 at 10:54 AM Zachary Zhou  wrote:
>
> This is sample code for reference
>
> HW support for decode+scaling in a single HW command (VDBOX+SFC).
> The primary target usage is video analytics, but can be used playback,
> transcoding, etc.
>
> For VAAPI -
> https://github.com/intel/libva
> basically, it allows multiple outputs (in different resolutions) using the 
> decode context in a single call (you can search for “additional_outputs” in 
> va.h).
>
> VAAPI sample code -
> https://github.com/intel/libva-utils/commit/957a269f02b00760b7e807643c821ee26abc529b
> ---
>  libavcodec/avcodec.h   |   8 +++
>  libavcodec/decode.c|  16 +
>  libavcodec/options_table.h |   4 ++
>  libavcodec/vaapi_decode.c  | 122 ++---
>  libavcodec/vaapi_decode.h  |  30 +
>  libavcodec/vaapi_h264.c|  13 
>  6 files changed, 185 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 0ce22ec4fa..36db21c0a5 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3357,6 +3357,14 @@ typedef struct AVCodecContext {
>   * - encoding: unused
>   */
>  int discard_damaged_percentage;
> +
> +/*
> + * Thumbnail options
> + */
> +int thumbnail_flags;
> +int thumbnail_format;
> +int thumbnail_width;
> +int thumbnail_height;
>  } AVCodecContext;
>

Global fields for such a purpose seem not appropriate. We try to get
away from fields that only serve a single purpose with a single
decoder. Nevermind that they aren't even documented.

In general I must say I'm not sure I like this entire patchset. We
have component seperation for a reason, so trying to add magic
features like that is generally a bad idea.

- 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 V1] lavf/matroskaenc: Fix memory leak after write trailer

2019-04-04 Thread Hendrik Leppkes
On Thu, Apr 4, 2019 at 3:05 PM Andreas Rheinhardt via ffmpeg-devel
 wrote:
>
> And I think that this memleak in mkv_write_trailer() has a twin in
> mkv_write_packet(): Audio frames are not written directly, but rather
> put into storage in cur_audio_pkt via av_packet_ref(). But if the
> preceding audio packet was a zero size packet (but possibly with
> side-data), then the preceding packet's side data will leak at this
> point. A better solution seems to be never to store a packet whose
> buffer has size zero in cur_audio_pkt. The only use we have for such
> packets is to use them for mkv_check_new_extra_data() and that has
> already been done at this point.
>

That seems like a good idea.

- 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 V1] lavf/matroskaenc: Fix memory leak after write trailer

2019-04-04 Thread Hendrik Leppkes
On Thu, Apr 4, 2019 at 11:25 AM Jun Zhao  wrote:
>
> From: Jun Zhao 
>
> Fix memory leak after write trailer for #7827
>
> Signed-off-by: Jun Zhao 
> ---
>  libavformat/matroskaenc.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index b9f99c4..22ba93a 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2571,13 +2571,13 @@ static int mkv_write_trailer(AVFormatContext *s)
>  // check if we have an audio packet cached
>  if (mkv->cur_audio_pkt.size > 0) {
>  ret = mkv_write_packet_internal(s, >cur_audio_pkt, 0);
> -av_packet_unref(>cur_audio_pkt);
>  if (ret < 0) {
>  av_log(s, AV_LOG_ERROR,
> "Could not write cached audio packet ret:%d\n", ret);
>  return ret;
>  }
>  }
> +av_packet_unref(>cur_audio_pkt);
>

Won't this leak instead when the error path above is triggered?
Also, whats in the packet if it has a size of 0?

- 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] tests: don't include TARGET_PATH in the sample path needlessly

2019-04-03 Thread Hendrik Leppkes
The transcode() helper function will already prepend the TARGET_PATH to
the sample path, if its a relative path. This avoids an issue on
Windows, where the relative path check could fail.
---
 tests/fate/ffmpeg.mak | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fate/ffmpeg.mak b/tests/fate/ffmpeg.mak
index af7282f9ab..71ab2f1f63 100644
--- a/tests/fate/ffmpeg.mak
+++ b/tests/fate/ffmpeg.mak
@@ -95,7 +95,7 @@ fate-copy-trac2211-avi: CMD = transcode "h264 -r 14" 
$(TARGET_SAMPLES)/h264/bbc2
 
 FATE_STREAMCOPY-$(call ENCDEC, APNG, APNG) += fate-copy-apng
 fate-copy-apng: fate-lavf-apng
-fate-copy-apng: CMD = transcode apng 
"$(TARGET_PATH)/tests/data/lavf/lavf.apng" apng "-c:v copy"
+fate-copy-apng: CMD = transcode apng tests/data/lavf/lavf.apng apng "-c:v copy"
 
 FATE_STREAMCOPY-$(call DEMMUX, OGG, OGG) += fate-limited_input_seek 
fate-limited_input_seek-copyts
 fate-limited_input_seek: $(TARGET_SAMPLES)/vorbis/moog_small.ogg
-- 
2.20.1.windows.1

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

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

Re: [FFmpeg-devel] [PATCH] avformat/file: add seekable option to disallow seeking

2019-04-03 Thread Hendrik Leppkes
On Wed, Apr 3, 2019 at 10:42 PM Marton Balint  wrote:
>
> Signed-off-by: Marton Balint 
> ---
>  doc/protocols.texi | 9 +
>  libavformat/file.c | 5 +
>  2 files changed, 14 insertions(+)
>
> diff --git a/doc/protocols.texi b/doc/protocols.texi
> index e1caa049a5..34967ff5e2 100644
> --- a/doc/protocols.texi
> +++ b/doc/protocols.texi
> @@ -199,6 +199,15 @@ If set to 1, the protocol will retry reading at the end 
> of the file, allowing
>  reading files that still are being written. In order for this to terminate,
>  you either need to use the rw_timeout option, or use the interrupt callback
>  (for API users).
> +
> +@item seekable
> +Controls if seekability is advertised on the file. 1 means seekable, 0 means
> +non-seekable, -1 means auto (seekable for normal files, non-seekable for 
> named
> +pipes).
> +
> +Many demuxers handle seekable and non-seekable resources differently,
> +overriding this might speed up opening certain files at the cost of losing 
> some
> +features (e.g. accurate seeking).
>  @end table
>
>  @section ftp
> diff --git a/libavformat/file.c b/libavformat/file.c
> index e613b91010..e46596fed1 100644
> --- a/libavformat/file.c
> +++ b/libavformat/file.c
> @@ -73,6 +73,7 @@ typedef struct FileContext {
>  int trunc;
>  int blocksize;
>  int follow;
> +int seekable;
>  #if HAVE_DIRENT_H
>  DIR *dir;
>  #endif
> @@ -82,6 +83,7 @@ static const AVOption file_options[] = {
>  { "truncate", "truncate existing files on write", offsetof(FileContext, 
> trunc), AV_OPT_TYPE_BOOL, { .i64 = 1 }, 0, 1, AV_OPT_FLAG_ENCODING_PARAM },
>  { "blocksize", "set I/O operation maximum block size", 
> offsetof(FileContext, blocksize), AV_OPT_TYPE_INT, { .i64 = INT_MAX }, 1, 
> INT_MAX, AV_OPT_FLAG_ENCODING_PARAM },
>  { "follow", "Follow a file as it is being written", 
> offsetof(FileContext, follow), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 1, 
> AV_OPT_FLAG_DECODING_PARAM },
> +{ "seekable", "Sets if the file is seekable", offsetof(FileContext, 
> seekable), AV_OPT_TYPE_INT, { .i64 = -1 }, -1, 1, AV_OPT_FLAG_DECODING_PARAM 
> | AV_OPT_FLAG_ENCODING_PARAM },
>  { NULL }
>  };
>
> @@ -238,6 +240,9 @@ static int file_open(URLContext *h, const char *filename, 
> int flags)
>  if (!h->is_streamed && flags & AVIO_FLAG_WRITE)
>  h->min_packet_size = h->max_packet_size = 262144;
>
> +if (c->seekable >= 0)
> +h->is_streamed = !c->seekable;
> +

You should probably not let a user enable seeking if the resource
doesn't allow it.

- 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 1/2] avcodec/h264_parse: change prefix to avpriv for usage in avformat mxf muxer

2019-04-03 Thread Hendrik Leppkes
On Wed, Apr 3, 2019 at 2:28 PM Carl Eugen Hoyos  wrote:
>
> 2019-04-03 11:37 GMT+02:00, Hendrik Leppkes :
>
> > We prefer not to expose huge modules like this as avpriv,
> > as it makes it part of the ABI
>
> (I am sure there are cases that are difficult to avoid,
> we also prefer not to duplicate functions.)
>
> > and as such impossible to change without
> > deprecation cycles.
>
> That does not sound correct to me.
>

You are correct, we don't need deprecation cycles. But we do need a
major bump to change it, so thats typically still a up to 2 year
delay.

- 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 1/2] avcodec/h264_parse: change prefix to avpriv for usage in avformat mxf muxer

2019-04-03 Thread Hendrik Leppkes
On Wed, Apr 3, 2019 at 11:22 AM Baptiste Coudurier
 wrote:
>
> ---
>  libavcodec/cbs_h2645.c | 28 ++--
>  libavcodec/extract_extradata_bsf.c |  4 ++--
>  libavcodec/h2645_parse.c   |  6 +++---
>  libavcodec/h2645_parse.h   |  6 +++---
>  libavcodec/h264_parse.c|  4 ++--
>  libavcodec/h264_parser.c   |  4 ++--
>  libavcodec/h264_ps.c   |  4 ++--
>  libavcodec/h264_ps.h   |  4 ++--
>  libavcodec/h264_slice.c|  2 +-
>  libavcodec/h264data.c  |  2 +-
>  libavcodec/h264data.h  |  3 ++-
>  libavcodec/h264dec.c   | 10 +-
>  libavcodec/hevc_parse.c|  2 +-
>  libavcodec/hevc_parser.c   |  4 ++--
>  libavcodec/hevcdec.c   |  4 ++--
>  libavcodec/svq3.c  |  2 +-
>  16 files changed, 45 insertions(+), 44 deletions(-)
>

We prefer not to expose huge modules like this as avpriv, as it makes
it part of the ABI and as such impossible to change without
deprecation cycles.
Preferably, avpriv should be avoided entirely where possible, as it
has many of the same limitations as actual public API, and just adds
confusion.

- 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 10/15] avformat/matroskaenc: Avoid seeking when writing level 1 elements

2019-04-02 Thread Hendrik Leppkes
On Tue, Apr 2, 2019 at 3:36 PM Andreas Rheinhardt via ffmpeg-devel
 wrote:
> @@ -383,8 +388,8 @@ static void end_ebml_master_crc32_preliminary(AVIOContext 
> *pb, AVIOContext **dyn
>  uint8_t *buf;
>  int size = avio_get_dyn_buf(*dyn_cp, );
>
> +put_ebml_num(pb, size, master.sizebytes);
>  avio_write(pb, buf, size);
> -end_ebml_master(pb, master);
>  }
>

The indent here seems off, or did my mail client mangle something?

- 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 10/15] avformat/matroskaenc: Avoid seeking when writing level 1 elements

2019-04-02 Thread Hendrik Leppkes
On Tue, Apr 2, 2019 at 5:55 PM Andreas Rheinhardt via ffmpeg-devel
 wrote:
>
> Hello,
>
> thanks for taking a look at this.
>
> Hendrik Leppkes:
> > On Tue, Apr 2, 2019 at 3:36 PM Andreas Rheinhardt via ffmpeg-devel
> >  wrote:
> >>
> >> Up until now, the writing process for level 1 elements (those elements
> >> for which CRC-32 elements are written by default) was this in case the
> >> output was seekable: Write the EBML ID, write an "unkown length" EBML
> >> number of the desired length, then write the element into a dynamic
> >> buffer, then write the dynamic buffer (after possible calculation and
> >> writing of the CRC-element), then seek back to the size element and
> >> overwrite the unknown-size element with the real size. The seeking and
> >> overwriting part has been eliminated by not writing the size initially.
> >>
> >
> > I'm not particularly happy that it stops using start_ebml_master and
> > basically duplicates its functionality. This adds possible maintenance
> > in the future, or hidden bugs.
> >
> 1. start_ebml_master has the disadvantage that the length of the size
> element is chosen in advance; if one doesn't want to use another
> dynamic buffer for every master element (I'm thinking about the
> thousands of cues for which this would be mostly useless as they are
> written with one byte length fields anyway) and doesn't want to waste
> bytes for writing lots of elements (the level 1 elements), then these
> two functions need to be separated.

I understand why its done, it just doesn't seem .. ideal.

I didn't check the code entirely, but can this function theoretically
still be used for non-L1 elements after your changes, if one is
desired to have a CRC32 further down the EBML tree?
If not, it might be sensible to rename it.

>
> 6. Or do you mean by making "relative positions valid again" that you
> want that the offset in s->pb + the offset of an element in the
> dynamic buffer coincides with the position where this element will
> actually by written in the output? This would effectively mean that
> one can't use the minimum required number of bytes for the cluster's
> size field and instead would have to reserve so many that it always
> works. The only advantage of this would be that the "Writing block at
> offset" log message can really spit out the real output offset of the
> block (which is impossible if the length of the cluster's size field
> hasn't been chosen before writing the block). That's a very slim
> advantage to me.
>

I forgot that relative positions  within an element are supposed to be
counted only from the size element, so nevermind that part.

- 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 10/15] avformat/matroskaenc: Avoid seeking when writing level 1 elements

2019-04-02 Thread Hendrik Leppkes
On Tue, Apr 2, 2019 at 3:36 PM Andreas Rheinhardt via ffmpeg-devel
 wrote:
>
> Up until now, the writing process for level 1 elements (those elements
> for which CRC-32 elements are written by default) was this in case the
> output was seekable: Write the EBML ID, write an "unkown length" EBML
> number of the desired length, then write the element into a dynamic
> buffer, then write the dynamic buffer (after possible calculation and
> writing of the CRC-element), then seek back to the size element and
> overwrite the unknown-size element with the real size. The seeking and
> overwriting part has been eliminated by not writing the size initially.
>

I'm not particularly happy that it stops using start_ebml_master and
basically duplicates its functionality. This adds possible maintenance
in the future, or hidden bugs.

Additionally, before this change, the position of the current writer
points to the position where the dynamic block is actually going to be
written - after this, it'll write the size inbetween.
Especially considering this behavior is different between seek and
non-seek, I feel a bit uneasy if something might want to reference the
position - there is a specific warning about that in the CRC case,
which would apply here equally.

Maybe the last point can be improved if the size is being included in
the dynamic buffer and overwritten therein, as such making relative
positions valid again, even if different to the non-seek case?


- 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 4/5] x86/opusdsp: implement FMA3 accelerated postfilter and deemphasis

2019-04-02 Thread Hendrik Leppkes
On Mon, Apr 1, 2019 at 4:59 PM James Almer  wrote:
>
> On 4/1/2019 9:13 AM, Lynne wrote:
> > Mar 31, 2019, 11:49 PM by jamr...@gmail.com:
> >
> >> A float ret value needs to be in xmm0, and you swapped m0 with m2 on
> >> Win64. This is the source of the fate failure.
> >>
> > Attached a patch to fix this.
> >
> >> %if WIN64
> >> -SWAP 0, 2
> >> -%endif
> >> +shufps m0, m2, 0
> >> +%else
> >>   shufps m0, m0, 0
> >> +%endif
> >> %endif
> >
> > 0001-x86-opusdsp-fix-WIN64-return-value.patch
> >
> > From 98e93b6f322a2a9dee7499fe87b74cf50a33b022 Mon Sep 17 00:00:00 2001
> > From: Lynne 
> > Date: Mon, 1 Apr 2019 13:06:34 +0100
> > Subject: [PATCH] x86/opusdsp: fix WIN64 return value
> >
> > ---
> >  libavcodec/x86/opusdsp.asm | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavcodec/x86/opusdsp.asm b/libavcodec/x86/opusdsp.asm
> > index ed65614e06..a7bff4f0b3 100644
> > --- a/libavcodec/x86/opusdsp.asm
> > +++ b/libavcodec/x86/opusdsp.asm
> > @@ -40,9 +40,10 @@ cglobal opus_deemphasis, 4, 4, 8, out, in, coeff, len
> >  VBROADCASTSS m0, coeffm
> >  %else
> >  %if WIN64
> > -SWAP 0, 2
> > -%endif
> > +shufps m0, m2, 0
>
> No, like this shufps will interleave the values from m2 and m0. The
> correct instruction would be
>
> shufps m0, m2, m2, 0
>
> Where m2 is used for both input arguments, and the VEX encoding lets us
> use another reg as output.
> Tested and pushed it with that change. Thanks.

Opus tests still fail on win32, both mingw and msvc.

- 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] fate: unbreak fate with custom binary names

2019-04-02 Thread Hendrik Leppkes
On Tue, Apr 2, 2019 at 1:27 PM Gyan  wrote:
>
> I ran full fate and some tests failed as the ffmpeg binary wasn't found.
> Suffixes weren't respected. Fixed.
>
> I can run fate only occasionally. Maybe one of the regular FATE
> submitters could set a suffix (and extension) to keep a check on this.
>

Might this be why fate-copy-apng fails on Windows? Since Windows uses
a suffix of course.

- 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] There may be a bug for .mp4 reader.

2019-04-01 Thread Hendrik Leppkes
On Mon, Apr 1, 2019 at 7:21 PM Yufei He  wrote:
>
> Hi
>
> There may be a bug for .mp4 reader.
>
> On decoding some ntsc mp4 files with my h.264 codec, from actvx I received,
>
> avctx->framerate.den is 100
>
> avctx->framerate.num is 2997
>
> avctx->pkt_timebase.num is 1
> avctx->pkt_timebase.den is 2997
>
> Duration of every packet I received from ff_decode_get_packet(avctx, ) is 
> 100, which means it's a encoded frame. But actually, it's a encoded field, 
> it's duration should be 50.
>

Bug reports should go on Trac or the API user mailing list.

- 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] [FFmpeg-cvslog] avcodec/libaomenc: fix default value for row-mt option

2019-03-30 Thread Hendrik Leppkes
On Sat, Mar 30, 2019 at 2:52 PM Gyan  wrote:
>
>
>
> On 30-03-2019 06:09 PM, Moritz Barsnick wrote:
> > On Sat, Mar 30, 2019 at 16:50:52 +0530, Gyan wrote:
> >> And what are the semantics of the user setting row_mt or enable-intrabc
> >> as -1?
> > The user doesn't set -1, they set 0 or 1 as bool. -1 helps to indicate
> > that nothing was set, and - in these cases - falls back to not setting
> > anything in the libary interface and choosing its own default.
>
> The commit changed both the default value as well as the allowed range.
> The option value parser evaluates AV_OPT_TYPE_BOOL same as
> AV_OPT_TYPE_INT, so user inputs aren't reduced to a binary evaluation.
> The latter means that the user can assertively set -1, as opposed to it
> just being the ctx inited value. The same wasn't done for
> enable-intrabc. I was wondering if the user may ever want to set -1
> manually, and if so, what that meant.
>

-1 means "auto" - in this case, let the encoder decide, if possible.
AV_OPT_TYPE_BOOL is not just a simple alias for INT, because it
accepts a variety of string options as well.

-1 = auto
0 = false,n,no,disable,disabled,off
1 = true,y,yes,enable,enabled,on

In addition to also accepting any numeric input.

- 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] avcodec/get_bits: unbreak get_bits_le() with cached reader

2019-03-26 Thread Hendrik Leppkes
On Tue, Mar 26, 2019 at 11:32 AM Carl Eugen Hoyos  wrote:
>
> 2019-03-26 11:13 GMT+01:00, Paul B Mahol :
> > Signed-off-by: Paul B Mahol 
> > ---
> >  libavcodec/get_bits.h   | 82 +
> >  libavcodec/utvideodec.c |  4 +-
> >  2 files changed, 60 insertions(+), 26 deletions(-)
>
> How can the issue be reproduced?
>

Try to play https://files.1f0.de/samples/ut-test-t2-umrg.mkv in a 64-bit build.

- 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] swscale: Remove duplicated code

2019-03-25 Thread Hendrik Leppkes
On Mon, Mar 25, 2019 at 11:35 AM Lauri Kasanen  wrote:
>
> On Mon, 25 Mar 2019 11:17:38 +0100
> Michael Niedermayer  wrote:
>
> > On Sun, Mar 24, 2019 at 01:04:51PM +0200, Lauri Kasanen wrote:
> > > In this function, the exact same clamping happens both in the if and 
> > > unconditionally.
> > >
> > > Signed-off-by: Lauri Kasanen 
> > > ---
> > >  libswscale/output.c | 14 --
> > >  1 file changed, 14 deletions(-)
> >
> > The removed code is the one that should stay, the other should be
> > removed.
> > one check for a rarely true condition should be faster than 4 checks
>
> Yes, I thought so too, but the commit that added the unconditional code
> says it fixes a bug...
>

Could it overflow so high that other bits then the one being checked
for are set? Perhaps another bit pattern with more high bits set would
solve that.

- 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 v2] avfilter/src_movie: change the deprecate API to new decode api

2019-03-23 Thread Hendrik Leppkes
> > @@ -524,17 +522,22 @@ static int movie_push_frame(AVFilterContext *ctx, 
> > unsigned out_id)
> >  return AVERROR(ENOMEM);
> >
> >  frame_type = st->st->codecpar->codec_type;
> > -switch (frame_type) {
> > -case AVMEDIA_TYPE_VIDEO:
> > -ret = avcodec_decode_video2(st->codec_ctx, frame, _frame, pkt);
> > -break;
> > -case AVMEDIA_TYPE_AUDIO:
> > -ret = avcodec_decode_audio4(st->codec_ctx, frame, _frame, pkt);
> > -break;
> > -default:
> > +if (frame_type == AVMEDIA_TYPE_VIDEO || frame_type == 
> > AVMEDIA_TYPE_AUDIO) {
>
> > +ret = avcodec_send_packet(st->codec_ctx, pkt);
> > +if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) {
> > +ret = 0;
> > +}
> > +if (ret >= 0) {
> > +ret = avcodec_receive_frame(st->codec_ctx, frame);
>
> This is doing one avcodec_receive_frame() for each
> avcodec_send_packet(): this is not how the new API is supposed to be
> used. If it was, there would not have been the need for a new API in the
> first place, would it?
>

Its indeed not ideal, and instead it should loop over receive_frame
until EAGAIN is returned. Otherwise you might lose frames when
send_packet doesnt actually accept a new one yet.

Additionally, the patch could still use quite some cleanup in the
AVPacket handling, since the new API will always consume a full
AVPacket, and any manual handling of partial packets is no longer
required, so this all can be removed as well further down in that
function.

- 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 1/2] avutil/opt: Add AV_OPT_TYPE_UINT16

2019-03-22 Thread Hendrik Leppkes
On Fri, Mar 22, 2019 at 4:01 PM  wrote:
>
> From: Nick Renieris 
>
> Signed-off-by: Nick Renieris 
> ---
>  libavutil/opt.c | 29 +++--
>  libavutil/opt.h |  1 +
>  2 files changed, 28 insertions(+), 2 deletions(-)
>

We really don't need this type. You can just change the type of your
variable to int and set size limits on the option.

- 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 v5] lavc/qsvenc: add BRC sliding window setting

2019-03-18 Thread Hendrik Leppkes
On Mon, Mar 18, 2019 at 7:29 AM Zhong Li  wrote:
>
> WinBRCMaxAvgKbps is to specify maximum average bitrate over a
> sliding window with size of WinBRCSize
>
> WinBRCMaxAvgKbps will be ignored in CBR mode and equal to TargetKbps.
>

How are these modes different to ffmpeg rc_max_rate in conjunction
with rc_buffer_size, which are typically used for a VBV-like buffering
scheme in H264/5, which also seems to be a sliding window as I
understand it?

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


Re: [FFmpeg-devel] Bug in YUV decoder

2019-03-15 Thread Hendrik Leppkes
On Fri, Mar 15, 2019 at 10:13 PM Ben Hutchinson  wrote:
>
> Also, on another note, why don't we have yuvj410p as a video format? Each
> chroma-subsampled versionof yuv (partial range YUV) format should have an
> equivalent chroma-subsampled version of yuvj (full range yuv). This would
> allow various experimental setups to be tested.
>

The J formats are deprecated, you should specify the color range
independently using the appropriate color_range parameter, and new
ones will not be added.

PS:
Please don't top post on this list.

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


Re: [FFmpeg-devel] [PATCH]lavc/qtrle: Do not use aligned writes for odd addresses.

2019-03-14 Thread Hendrik Leppkes
On Fri, Mar 15, 2019 at 12:26 AM Carl Eugen Hoyos  wrote:
>
> 2019-03-15 0:13 GMT+01:00, Hendrik Leppkes :
> > On Fri, Mar 15, 2019 at 12:05 AM Carl Eugen Hoyos 
> > wrote:
> >>
> >> Hi!
> >>
> >> Attached patch fixes the qtrle crash on sparc for me.
> >>
> >
> > It should be fine in cases where the pointer is being incremented by
> > an aligned amount, ie. writing 32 and incrementing by 4, or 64 and 8,
> > etc.
>
> Of course, thank you for the review!
> New patch attached.
>

LGTM if tested.

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


Re: [FFmpeg-devel] [PATCH]lavc/qtrle: Do not use aligned writes for odd addresses.

2019-03-14 Thread Hendrik Leppkes
On Fri, Mar 15, 2019 at 12:05 AM Carl Eugen Hoyos  wrote:
>
> Hi!
>
> Attached patch fixes the qtrle crash on sparc for me.
>

It should be fine in cases where the pointer is being incremented by
an aligned amount, ie. writing 32 and incrementing by 4, or 64 and 8,
etc.
Are all of those required to pass, or only the ones on "odd"
increments like 3 for 24-bit RGB? (In fact, those "odd" ones are the
ones that are new recently)

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


Re: [FFmpeg-devel] fate/proresenc_aw : Add fate test for interlace and 444 encoding

2019-03-07 Thread Hendrik Leppkes
On Wed, Feb 27, 2019 at 6:05 PM Martin Vignali  wrote:
>
> > works here
> >
> >
>  Pushed, thanks.
>

These tests fail on all FATE boxes. http://fate.ffmpeg.org/

Can you check that and fix it?

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


Re: [FFmpeg-devel] [PATCH] avcodec/nvenc: Reconfigure resolution on-the-fly

2019-03-06 Thread Hendrik Leppkes
On Wed, Mar 6, 2019 at 3:57 PM Oliver Collyer  wrote:
>
> diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
> index 0ce22ec4fa..7087f82ce1 100644
> --- a/libavcodec/avcodec.h
> +++ b/libavcodec/avcodec.h
> @@ -3357,6 +3357,12 @@ typedef struct AVCodecContext {
>   * - encoding: unused
>   */
>  int discard_damaged_percentage;
> +
> +/*
> + * Video encoding only. Sets the maximum picture size for encoders that
> + * support adjusting the picture size dynamically during encoding.
> + */
> + int max_width, max_height;
>  } AVCodecContext;
>

I really don't like introducing public API fields for this. Maybe a
private nvenc option would be better at this point.

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


Re: [FFmpeg-devel] [PATCH] apedec: add ability to check CRC

2019-03-06 Thread Hendrik Leppkes
On Wed, Mar 6, 2019 at 2:18 PM Carl Eugen Hoyos  wrote:
>
> 2019-03-06 12:22 GMT+01:00, Lynne :
> > The CRC flag is only signalled once every few minutes but CRC is still
> > always present so the patch uses the file version instead.
> > CRC on 24-bit files wants non-padded samples so skip such files.
> > Some corrupt samples may have been output before the final check
> > depending on the -max_samples setting.
>
> Imo, you should only return an error for some non-default
> strict value (it should be possible for the user to show the
> issue but continue to decode).
>

strict is the wrong value to check here. It could use err_recognition
& AV_EF_EXPLODE and only print otherwise.

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


Re: [FFmpeg-devel] [PATCH] doc: add missing hyphen prefix

2019-02-22 Thread Hendrik Leppkes
On Fri, Feb 22, 2019 at 12:29 PM Nicolas George  wrote:
>
> Lou Logan (12019-02-21):
> > For consistency. Fixes #7740.
> >
> > Signed-off-by: Lou Logan 
>
> I do not think this is correct: the dash is not part of the option name,
> it is part of the Unix command-line tradition. For consistency, it
> should be removed when it is there, possibly replaced by the word
> "option" if necessary.
>

I agree. You don't pass the dash when yo use avoptions through API,
for example, so this would only add to the confusion.

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


Re: [FFmpeg-devel] [PATCH]lavf/spdifenc: Use a more flexible buffer model for TrueHD/MAT

2019-02-21 Thread Hendrik Leppkes
On Thu, Feb 21, 2019 at 3:28 PM Carl Eugen Hoyos  wrote:
>
> 2019-02-21 10:40 GMT+01:00, Hendrik Leppkes :
>
> > https://files.1f0.de/samples/incredibles2-truehd-bitstreaming.thd
> > (TrueHD stream)
> > https://files.1f0.de/samples/incredibles2-truehd-bitstreaming.pcm.xz
> > (bitstreaming output, xz compressed)
>
> 404 =-(
>

Sorry about that, fixed the files, should work now.

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


Re: [FFmpeg-devel] [PATCH]lavf/spdifenc: Use a more flexible buffer model for TrueHD/MAT

2019-02-21 Thread Hendrik Leppkes
On Fri, Feb 15, 2019 at 1:13 AM Carl Eugen Hoyos  wrote:
>
> > - The distance between frames can be incredibly large in some streams,
> > I've seen it be larger then one whole MAT frame, and the output works
> > reliably (MAT start/middle/end codes need to be placed within the
> > padding space, of course).
>
> That is not supported yet, could you provide such a problematic sample?
> (Including the output if possible)
>

I didn't want to cut it too badly, since that can disrupt how it
works, so its a bit sizeable (the bitrate is also quite high), its the
first ~4 minutes from the start of the Incredibles 2 UHD Blu-ray

https://files.1f0.de/samples/incredibles2-truehd-bitstreaming.thd
(TrueHD stream)
https://files.1f0.de/samples/incredibles2-truehd-bitstreaming.pcm.xz
(bitstreaming output, xz compressed)

There should be a few occurances of this particular case, its the same
stream I used to look into these problems in the first place after it
was reported to me by users.

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


Re: [FFmpeg-devel] [PATCH]lavf/spdifenc: Use a more flexible buffer model for TrueHD/MAT

2019-02-14 Thread Hendrik Leppkes
On Thu, Feb 14, 2019 at 8:11 PM Carl Eugen Hoyos  wrote:
>
> Hi!
>
> I created attached patch with a lot of help from Hendrik, fixes ticket #7731.
>

Som general comments, since I can't replay inline in this mail client:

- The pseudo-timestamps are unsigned 16-bit, personally I would make
all types that reference them uint16_t and rely on unsigned overflow
behavior, which is fully valid and specified - no need to handle
negatives manually, etc.
- The distance between frames can be incredibly large in some streams,
I've seen it be larger then one whole MAT frame, and the output works
reliably (MAT start/middle/end codes need to be placed within the
padding space, of course).

There is a few more things I saw, but I'll respond to those from a
better client in a few days when I'm back home.

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


Re: [FFmpeg-devel] [FFmpeg-cvslog] avutil/cuda_check: avoid pointlessly exporting same symbol from two libraries

2019-02-14 Thread Hendrik Leppkes
On Thu, Feb 14, 2019 at 11:36 PM Carl Eugen Hoyos  wrote:
> >>> No, this entire mess with duplicated ff_ symbols is specifically to
> >>> avoid having to include it in the ABI.
> >>
> >> But old libavcodec does not work with new libavutil now or am I wrong?
> >
> > Is that really a thing we expect or advertise to work? It does not seem
> > sane and I'd expect a lot of other things to explode.
>
> We don't "advertise" it but it is certainly expected from any half-sane
> project.

Its not a problem in any "normal" case. No ff_ symbols are exported,
so dynamic linking would never use it.
What might not work in the future is mixing static libraries of
different versions, but anyone that does something like that is truely
insane anyway.

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


Re: [FFmpeg-devel] [PATCH]lavf/spdifenc: Use a more flexible buffer model for TrueHD/MAT

2019-02-14 Thread Hendrik Leppkes
On Thu, Feb 14, 2019 at 8:14 PM Carl Eugen Hoyos  wrote:
>
> 2019-02-14 20:11 GMT+01:00, Carl Eugen Hoyos :
>
> > I created attached patch with a lot of help from Hendrik, fixes
> > ticket #7731.
>
> I forgot: I still need help to fix the frame distance calculation,
> how is the frame rate supposed to be used?
> Iiuc, the current code only works for 48kHz audio.
>

You want to shift the 64 to the right by the ratebits from the TrueHD
header, see ff_mlp_read_major_sync where to find it.

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


Re: [FFmpeg-devel] [FFmpeg-cvslog] avutil/cuda_check: avoid pointlessly exporting same symbol from two libraries

2019-02-14 Thread Hendrik Leppkes
On Thu, Feb 14, 2019 at 4:51 PM Carl Eugen Hoyos  wrote:
>
>
>
> > Am 14.02.2019 um 13:39 schrieb Timo Rothenpieler :
> >
> > ffmpeg | branch: master | Timo Rothenpieler  | Fri 
> > Feb  8 22:47:01 2019 +0100| [15c6390139096b7e7634bf0f6aaab1cd8b3aa509] | 
> > committer: Timo Rothenpieler
> >
> > avutil/cuda_check: avoid pointlessly exporting same symbol from two 
> > libraries
> >
> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=15c6390139096b7e7634bf0f6aaab1cd8b3aa509
> > ---
> >
> > libavcodec/Makefile |  6 +++---
> > libavcodec/cuda_check.c |  1 -
> > libavutil/Makefile  |  2 +-
>
> > libavutil/cuda_check.c  | 45 -
>
> Apart from breaking compilation doesn’t this also break ABI?
>

No, this entire mess with duplicated ff_ symbols is specifically to
avoid having to include it in the ABI.

The compilation error should of course be fixed.

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


Re: [FFmpeg-devel] [PATCH]lavf/spdifenc: Automatically insert truehd_core bitstream filter

2019-02-12 Thread Hendrik Leppkes
On Wed, Feb 13, 2019 at 2:57 AM Carl Eugen Hoyos  wrote:
>
> 2019-02-13 0:47 GMT+01:00, Hendrik Leppkes :
> > On Tue, Feb 12, 2019 at 12:54 PM Carl Eugen Hoyos 
> > wrote:
> >>
> >> 2019-02-12 12:37 GMT+01:00, Hendrik Leppkes :
> >> > On Tue, Feb 12, 2019 at 12:28 PM Carl Eugen Hoyos 
> >> > wrote:
> >> >>
> >> >> Hi!
> >> >>
> >> >> Attached patch intends to fix ticket #7731.
> >> >
> >> > This is not a fix. The vast majority of TrueHD Atmos tracks work just
> >> > fine with the current limitations, and would needlessly drop the Atmos
> >> > information for every stream.
> >>
> >> Is it possible to detect if the Atmos core has to be dropped?
> >
> > Not beforehand, since the size of future frames is of course unknown,
> > and there are no indications in the bitstream.
> > One could consider starting to drop Atmos data after it happened once,
> > but it'll probably still glitch out audio at that point.
> >
> > Ideally the truehd spdif muxer should be revised to support a more
> > flexible buffering model, but its a tad bit complicated with the way
> > the spdif muxer is setup, and I've only recently learned myself how
> > its presumably supposed to be done, since the specifications are not
> > openly available.
>
> I did a few experiments before reading your mail:
> If I use a burst rate of significantly less than 2000 bytes
> audio gets broken with my receiver.
> Can you confirm that in any way?
> Otoh, it does not seem to help to insert memset(0)
> between frames if the burstrate is too low.
> ("burstrate": gap between beginnings of thd frames)
>
> It is not necessary to put 12 frames in both half-MAT frames,
> 15 and 9 work fine here.
>
> My receiver always fails / produces hickups if the thd stream
> contains atmos data: Are you sure it is supposed to work?

With every hardware? Who knows. My receiver does not support Atmos
either and it plays streams that exceed the 2560 size just fine with
correct muxing into MAT frames - although I haven't tested that one in
the ticket specifically I don't think, and I'm not near that receiver
until next week.

> Can you already provide a test stream for the audio stream
> from the ticket?
>

Sure, why not, although I have no idea how one would play this, since
you would need to make sure full MAT frames are always read and output
as one (ie. every 61440 bytes), and unfortunately our raw PCM demuxer
does not support specifying a wanted packet size, oh well.
https://files.1f0.de/tmp/truehd_11mbit_bug.spdif

Otherwise it should fit the typical TrueHD bitstreaming criteria, ie.
192kHz 8ch 16-bit "PCM", 61440 bytes frame size.

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


Re: [FFmpeg-devel] [PATCH]lavf/spdifenc: Automatically insert truehd_core bitstream filter

2019-02-12 Thread Hendrik Leppkes
On Tue, Feb 12, 2019 at 12:54 PM Carl Eugen Hoyos  wrote:
>
> 2019-02-12 12:37 GMT+01:00, Hendrik Leppkes :
> > On Tue, Feb 12, 2019 at 12:28 PM Carl Eugen Hoyos 
> > wrote:
> >>
> >> Hi!
> >>
> >> Attached patch intends to fix ticket #7731.
> >
> > This is not a fix. The vast majority of TrueHD Atmos tracks work just
> > fine with the current limitations, and would needlessly drop the Atmos
> > information for every stream.
>
> Is it possible to detect if the Atmos core has to be dropped?
>

Not beforehand, since the size of future frames is of course unknown,
and there are no indications in the bitstream.
One could consider starting to drop Atmos data after it happened once,
but it'll probably still glitch out audio at that point.

Ideally the truehd spdif muxer should be revised to support a more
flexible buffering model, but its a tad bit complicated with the way
the spdif muxer is setup, and I've only recently learned myself how
its presumably supposed to be done, since the specifications are not
openly available.

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


Re: [FFmpeg-devel] [PATCH]lavf/spdifenc: Automatically insert truehd_core bitstream filter

2019-02-12 Thread Hendrik Leppkes
On Tue, Feb 12, 2019 at 12:28 PM Carl Eugen Hoyos  wrote:
>
> Hi!
>
> Attached patch intends to fix ticket #7731.
>

This is not a fix. The vast majority of TrueHD Atmos tracks work just
fine with the current limitations, and would needlessly drop the Atmos
information for every stream.

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


Re: [FFmpeg-devel] [PATCH] avcodec/ac3: Explicitly return to discard large amounts of nonsense bytes

2019-02-06 Thread Hendrik Leppkes
On Thu, Jan 31, 2019 at 1:25 AM Michael Niedermayer
 wrote:
>
> Changes 19sec to 10ms (12559) runtime, 17sec to 177ms (12570)
> Fixes: Timeout
> Fixes: 
> 12559/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AC3_fuzzer-5666516266123264
> Fixes: 
> 12561/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_AC3_FIXED_fuzzer-5682923041193984
> Fixes: 
> 12570/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_EAC3_fuzzer-5194734308425728
>
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavcodec/ac3dec.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/ac3dec.c b/libavcodec/ac3dec.c
> index f844a463ee..eaa327a3ee 100644
> --- a/libavcodec/ac3dec.c
> +++ b/libavcodec/ac3dec.c
> @@ -1490,6 +1490,8 @@ static int ac3_decode_frame(AVCodecContext * avctx, 
> void *data,
>  }
>  if (i >= buf_size)
>  return AVERROR_INVALIDDATA;
> +if (i > 10)
> +return i;
>  buf += i;
>  buf_size -= i;
>

How exactly does this speed up the decoder, anyway? Isn't the garbage
immediately skipped right after this return?

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


Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

2019-02-06 Thread Hendrik Leppkes
On Thu, Jan 31, 2019 at 11:29 PM Chris Cunningham
 wrote:
>
> On Wed, Jan 30, 2019 at 5:43 PM James Almer  wrote:
>
> > On 1/30/2019 10:20 PM, Chris Cunningham wrote:
> > >>
> >  And this definitely means there's a bug elsewhere. This parser should
> >  have not been used for codecs ids other than GSM and GSM_MS. That's
> >  precisely what this assert() is making sure of.
> > 
> > >>>
> > >>> I'll take a closer look at how this parser was selected.
> > >>
> > >
> > > Ok, here are full details of how this happens:
> > >
> > >1. AV_CODEC_ID_GSM_MS is assigned to on st->codecpar->codec_id
> > >in ogm_header() (oggparseogm.c:75)
> > >2. The (deprecated) st->codec->codec_id is not assigned at that time
> > and
> > >remains 0 (UNKNOWN)
> > >3. AV_CODEC_ID_GSM_MS from st->codecpar is passed to av_parser_init,
> > >selecting the gsm parser (in read_frame_internal())
> > >4. The fuzzer next (only) packet has a size of 0, so the first call to
> > >parse_packet() (still in read_frame_internal()) does nothing
> > >5. After this call we assign several members of st->internal->avctx to
> > >st->codecpar. This leaves codecpar->codec_id = UNKNOWN.
> > Interestingly, we
> > >do not reset the st->parser at this point (maybe we should?)
> >
> > Where does this happen? A call to avcodec_parameters_from_context()
> > should also copy codec_id.
> >
>
> Ignore #5 here - I'm not seeing that today - was likely confused.
>
>
> >
> > >6. Next iteration of the read_frame_internal() loop we get EOF from
> > >ff_read_packet. This triggers the "flush the parsers" call to
> > >parse_packet() which finally reaches gsm_parse() to trigger the abort
> > >(avctx->codec_id is still 0).
> > >
> > > Questions (guessing at the fix):
> > >
> > >- At what point should st->codec->codec_id be synchronized with
> > >st->codecpar->codec_id? It never is in this test.
> >
> > In avformat_find_stream_info(), i think. In any case, st->codec should
> > have no effect on parsers. What is used in parse_packet() however is
> > st->internal->avctx, so that context is the one that evidently contains
> > codec_id == UNKNOWN when it should be in sync with codecpar.
> >
>
> We do call avformat_find_stream_info, and avcodec_parameters_from_context
> is called during that process. Everything seems OK when
> avformat_find_stream_info is done. The codecpar->codec_id is in sync with
> internal->avctx->codec_id for all streams.
>
> But then we start calling av_read_frame as part of normal demuxing.
> avcodec_parameters_from_context() does not appear to be called during this
> process. Eventually we get this stack:
>
> ogm_header
> ogg_packet
> ogg_read_packet
> ff_read_packet
> read_frame_internal
> av_read_frame
>
> Inside ogm_header, st->codecpar->codec_id is assigned AV_CODEC_ID_GSM_MS
> (previous value was codec NONE). But *st->internal->avctx->codec_id is
> never updated. It remains NONE for the rest of this test. *
>
> When this unwinds back to read_frame_internal, st->parser is assigned using
> this new codec (GSM_MS)
> st->parser = av_parser_init(st->codecpar->codec_id);
>
> Later on, still inside read_frame_internal's loop, we get EOF and call,
> parse_packet (/* flush the parsers */)
> parse_packet() calls av_parser_parse2(), passing st->internal->avctx
> (codec_id still NONE, while codecpar still says GSM_MS). This ultimately
> gets passed to gsm_parse, which triggers the assert0.
>
> The underlined bit above seems like the root cause. Where should we be
> updating st->internal->avctx->codec_id?

We have a flag to set that causes avformat to fix its internal state
if a demuxer changes it after the initial opening.
st->internal->need_context_update = 1

Try setting that at the position where the demuxer changes codecpar
(ie. in ogm_header?), and see if that resolves it.

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


Re: [FFmpeg-devel] [PATCH] configure: request explicitly enabled components

2019-02-05 Thread Hendrik Leppkes
On Tue, Feb 5, 2019 at 2:43 AM Carl Eugen Hoyos  wrote:
>
> 2019-02-05 1:13 GMT+01:00, Hendrik Leppkes :
> > On Tue, Feb 5, 2019 at 1:01 AM Carl Eugen Hoyos  wrote:
> >>
> >> 2019-02-05 0:53 GMT+01:00, Marton Balint :
> >> >
> >> >
> >> > On Tue, 5 Feb 2019, Carl Eugen Hoyos wrote:
> >> >
> >> >> 2019-02-03 16:24 GMT+01:00, Marton Balint :
> >> >>>
> >> >>>
> >> >>> On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote:
> >> >>>
> >> >>>> 2019-01-28 2:00 GMT+01:00, Marton Balint :
> >> >>>>> If we enable a component but a dependant library is disabled, then
> >> >>>>> the
> >> >>>>> enabled
> >> >>>>> component get silently disabled. Requesting all explicitly enabled
> >> >>>>> components
> >> >>>>> allows configure to fail and show the missing dependencies instead
> >> >>>>> of
> >> >>>>> ignoring
> >> >>>>> our request.
> >> >>>>>
> >> >>>>> For example if libdav1d is not availble ./configure
> >> >>>>> --enable-decoder=libdav1d
> >> >>>>> succeeds but the libdav1d decoder will not be enabled. After the
> >> >>>>> patch
> >> >>>>> the
> >> >>>>> configure line will fail with the following message:
> >> >>>>> ERROR: libdav1d_decoder requested, but not all dependencies are
> >> >>>>> satisfied:
> >> >>>>> libdav1d
> >> >>>>>
> >> >>>>> Signed-off-by: Marton Balint 
> >> >>>>> ---
> >> >>>>>  configure | 1 +
> >> >>>>>  1 file changed, 1 insertion(+)
> >> >>>>>
> >> >>>>> diff --git a/configure b/configure
> >> >>>>> index e1412352fa..1f6c6a7311 100755
> >> >>>>> --- a/configure
> >> >>>>> +++ b/configure
> >> >>>>> @@ -3881,6 +3881,7 @@ for opt do
> >> >>>>>  list=$(filter "$name" $list)
> >> >>>>>  [ "$list" = "" ] && warn "Option $opt did not match
> >> >>>>> anything"
> >> >>>>>  $action $list
> >> >>>>
> >> >>>>> +test $action = enable && request $list
> >> >>>>
> >> >>>> I strongly suspect that this will break regression tests.
> >> >>>
> >> >>> You mean fate with different configure options?
> >> >>
> >> >> No, I believe this would break regression tests with
> >> >> --disable-everything (and an enable for a feature that
> >> >> was added in the meantime and is needed to reproduce
> >> >> the issue).
> >> >
> >> > Could you give a more concrete example? I am not sure I
> >> > understand what you mean.
> >>
> >> $ ./configure --disable-everything --enable-bsf=prores_metadata
> >> currently does not fail for current FFmpeg and 936d18fb, this
> >> would change for future new features with your patch.
> >>
> >
> > What sort of testing would involve trying to enable a certain
> > component, and then *succeeding* when the component does not exist?
>
> Several regression tests have needed that in the past, I find it
> strange that you seem to imply this will not happen in the
> future.
> Or is that not what you were saying?
>

I'm trying to understand what type of test you are running that would
need this. Please explain isntead of saying "something needs this".
It just confuses me that any test would be meaningful if you try to
enable a component and end up with a build without it even present.

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


Re: [FFmpeg-devel] [PATCH] configure: request explicitly enabled components

2019-02-04 Thread Hendrik Leppkes
On Tue, Feb 5, 2019 at 1:01 AM Carl Eugen Hoyos  wrote:
>
> 2019-02-05 0:53 GMT+01:00, Marton Balint :
> >
> >
> > On Tue, 5 Feb 2019, Carl Eugen Hoyos wrote:
> >
> >> 2019-02-03 16:24 GMT+01:00, Marton Balint :
> >>>
> >>>
> >>> On Sun, 3 Feb 2019, Carl Eugen Hoyos wrote:
> >>>
>  2019-01-28 2:00 GMT+01:00, Marton Balint :
> > If we enable a component but a dependant library is disabled, then the
> > enabled
> > component get silently disabled. Requesting all explicitly enabled
> > components
> > allows configure to fail and show the missing dependencies instead of
> > ignoring
> > our request.
> >
> > For example if libdav1d is not availble ./configure
> > --enable-decoder=libdav1d
> > succeeds but the libdav1d decoder will not be enabled. After the patch
> > the
> > configure line will fail with the following message:
> > ERROR: libdav1d_decoder requested, but not all dependencies are
> > satisfied:
> > libdav1d
> >
> > Signed-off-by: Marton Balint 
> > ---
> >  configure | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/configure b/configure
> > index e1412352fa..1f6c6a7311 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3881,6 +3881,7 @@ for opt do
> >  list=$(filter "$name" $list)
> >  [ "$list" = "" ] && warn "Option $opt did not match
> > anything"
> >  $action $list
> 
> > +test $action = enable && request $list
> 
>  I strongly suspect that this will break regression tests.
> >>>
> >>> You mean fate with different configure options?
> >>
> >> No, I believe this would break regression tests with
> >> --disable-everything (and an enable for a feature that
> >> was added in the meantime and is needed to reproduce
> >> the issue).
> >
> > Could you give a more concrete example? I am not sure I
> > understand what you mean.
>
> $ ./configure --disable-everything --enable-bsf=prores_metadata
> currently does not fail for current FFmpeg and 936d18fb, this
> would change for future new features with your patch.
>

What sort of testing would involve trying to enable a certain
component, and then *succeeding* when the component does not exist? If
anything, you would make it fail sooner, before any actual testing
that would presumably actually fail later (otherwise, what would be
the point exactly?)

Please elaborate more then one random command line that without
explanation does not really make sense. If I request a certain
component, I would expect one of two things: I get an error, or I get
a build that includes that component. Anything else would be
unexpected, and therefor bad.
We have this wanted behavior with other options already, just not this
one, so its  not even consistent at all.

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


Re: [FFmpeg-devel] Access to the dolby vision decoder info

2019-02-04 Thread Hendrik Leppkes
On Mon, Feb 4, 2019 at 5:04 PM Gresserman Dmitrij
 wrote:
>
> I fully agree with your approach and already thought about this. Parsing this 
> directly to a structure would the correct approach for a final future patch. 
> Unfortunately at this point in development, i need the most flexibility 
> possible, with simultaneously minimal reaction time.
>

Since adding new types like this is basically public API, with certain
stability guarantees once added, we prefer proper solutions, instead
of quick hacks, otherwise we'll be dragging it around for years to
come.

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


Re: [FFmpeg-devel] Access to the dolby vision decoder info

2019-02-01 Thread Hendrik Leppkes
On Fri, Feb 1, 2019 at 8:42 PM Gresserman Dmitrij
 wrote:
>
> The main motivation of the patch is to provide the complete structure of the 
> Dolby Vision Configuration Box and decoder configuration record (dvcC/dvvC) 
> and the DOVI Video Stream Descriptor. This provided structure can be parsed 
> now and used (for example in dump.c) to indicate if a dolby vision stream is 
> part of the analyzed file. Furthermore the profile and type of the dolby 
> vsion sub streams can be displayed now.
>
> The information of the origin is needed to parse the provided structure. The 
> Descriptor and the Boxes have at this point nearly equal structures, but the 
> reserved size varies significantly. So it may become releveant to know the 
> origin of the provided structure in the future, therefore i added the 
> prefixes.
>

We try to define full structures for our side-data types, and not just
copy opaque info from the bitstream into it.
So I would suggest to actually create a struct for it (in avutil,
similar to other HDR metadata), and parsing both blocks into the
structurure. This would make using and understanding the structs from
API users much easier.

side data structures are also designed to be extensible in the future,
so if new fields are being added from the reserved space, we can
extend the structures and add them.

This would then automatically resolve any ambiguity between the
structs from different containers.

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


Re: [FFmpeg-devel] [PATCH] Support HDR dynamic metadata (HDR10+) in HEVC decoder.

2019-02-01 Thread Hendrik Leppkes
On Tue, Jan 22, 2019 at 11:13 PM Mohammad Izadi  wrote:
>
> ---
>  libavcodec/hevc_sei.c | 216 --
>  libavcodec/hevc_sei.h |   6 ++
>  libavcodec/hevcdec.c  |  21 
>  3 files changed, 237 insertions(+), 6 deletions(-)
>

Whats the status of this patch now? I've been sort-of waiting for it
to land, so I can start experimenting with the metadata.

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


Re: [FFmpeg-devel] [FFmpeg-cvslog] avcodec/vdpau: Enable HEVC support for working Nvidia driver versions

2019-01-27 Thread Hendrik Leppkes
On Sun, Jan 27, 2019 at 7:43 PM Dominik 'Rathann' Mierzejewski
 wrote:
>
> If you're trying to say we can upgrade 4.0.x to 4.1.x without
> recompiling any dependent packages and you guarantee everything will
> work just like with 4.0.x, then I would be willing to entertain that
> option, though we try to avoid upgrading base system components in the
> middle of a release cycle.
>

Whats more base system then a driver? :)

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


Re: [FFmpeg-devel] [PATCH 2/5] lavc/qsvdec: Replace current parser with MFXVideoDECODE_DecodeHeader()

2019-01-25 Thread Hendrik Leppkes
On Fri, Jan 25, 2019 at 11:43 AM Li, Zhong  wrote:
> avctx->pix_fmt should be get from MFXVideoDECODE_DecodeHeader() but it is 
> needed to init MSDK session, and looks like a deadlock, :(
> One solution is that we assume avctx->format has been parsed somewhere else 
> (such as avformat_find_stream_info() form libavformt) before start decoding.
>

You cannot rely on that. Especially since noone knows what format the
decoder is going to use. avformat will tell you its YUV420P10, not
P010.
A user who doesn't use avformat at all and only feed avcodec will
likely not set avctx->format at all, since basically nothing else
needs it.

A decoder has to figure out the pixel format by itself.

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


Re: [FFmpeg-devel] [PATCH 1/3] avutil/attributes: add av_likely and av_unlikely

2019-01-24 Thread Hendrik Leppkes
On Thu, Jan 24, 2019 at 11:06 PM Michael Niedermayer
 wrote:
>
> On Thu, Jan 24, 2019 at 06:08:57PM -0300, James Almer wrote:
> > On 1/24/2019 5:53 PM, Rostislav Pehlivanov wrote:
> > > On Thu, 24 Jan 2019 at 20:38, Marton Balint  wrote:
> > >
> > >> Signed-off-by: Marton Balint 
> > >> ---
> > >>  doc/APIchanges | 3 +++
> > >>  libavutil/attributes.h | 8 
> > >>  libavutil/version.h| 2 +-
> > >>  3 files changed, 12 insertions(+), 1 deletion(-)
> > >>
> > >> diff --git a/doc/APIchanges b/doc/APIchanges
> > >> index a39a3ff2ba..38b5b980a6 100644
> > >> --- a/doc/APIchanges
> > >> +++ b/doc/APIchanges
> > >> @@ -15,6 +15,9 @@ libavutil: 2017-10-21
> > >>
> > >>  API changes, most recent first:
> > >>
> > >> +2019-01-xx - xx - lavu 56.27.100 - attributes.h
> > >> +  Add av_likely and av_unlikely
> > >> +
> > >>  2019-01-08 - xx - lavu 56.26.100 - frame.h
> > >>Add AV_FRAME_DATA_REGIONS_OF_INTEREST
> > >>
> > >> diff --git a/libavutil/attributes.h b/libavutil/attributes.h
> > >> index ced108aa2c..60972e5109 100644
> > >> --- a/libavutil/attributes.h
> > >> +++ b/libavutil/attributes.h
> > >> @@ -164,4 +164,12 @@
> > >>  #define av_noreturn
> > >>  #endif
> > >>
> > >> +#if AV_GCC_VERSION_AT_LEAST(2,96) || defined(__clang__)
> > >> +# define av_likely(x)  __builtin_expect(!!(x), 1)
> > >> +# define av_unlikely(x)__builtin_expect(!!(x), 0)
> > >> +#else
> > >> +# define av_likely(x)  (x)
> > >> +# define av_unlikely(x)(x)
> > >> +#endif
> > >> +
> > >>  #endif /* AVUTIL_ATTRIBUTES_H */
> > >> diff --git a/libavutil/version.h b/libavutil/version.h
> > >> index 1fcdea95bf..12b4f9fc3a 100644
> > >> --- a/libavutil/version.h
> > >> +++ b/libavutil/version.h
> > >> @@ -79,7 +79,7 @@
> > >>   */
> > >>
> > >>  #define LIBAVUTIL_VERSION_MAJOR  56
> > >> -#define LIBAVUTIL_VERSION_MINOR  26
> > >> +#define LIBAVUTIL_VERSION_MINOR  27
> > >>  #define LIBAVUTIL_VERSION_MICRO 100
> > >>
> > >>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, 
> > >> \
> > >> --
> > >> 2.16.4
> > >>
> > >> ___
> > >> ffmpeg-devel mailing list
> > >> ffmpeg-devel@ffmpeg.org
> > >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > >
> > >
> > > NAK, NAK, NAK.
> > > I will NAK the hell out of any patch that does that. They're completely
> > > unnecessary, they're commonly used by complete idiots who add them 
> > > thinking
> > > their code will go faster (and it might - only on their antiquated GCC
> > > 3.1), they're religiously used by the same people and won't back down on
> > > using them and finally they're ugly when added to every single bloody
> > > branch and the people who maintain such code will demand they be added to
> > > every single bloody branch for no reason other that what I've just 
> > > iterated
> > > on.
> > > The ONLY way I'll accept them is in platform-specific files, e.g.
> > > libavcodec/ppc/*, and even then only on non-x86 arches (which need them 
> > > for
> > > having bad compilers with primitive processors having awful branch
> > > prediction) and even then I'd never accept their inclusioin into
> > > system-installed headers.
> >
> > What about hot loops with branches where you can use these as a hint for
> > the compiler to assume a specific outcome is highly unlikely to happen,
> > like alloc errors, corner cases in some codec, etc, which it simply has
> > no way to correctly guess at compile time?
> >
> > I don't think it should be NAKed because it's misused in other projects.
> > We're not other projects. We should instead simply ask the patch writer
> > to provide numbers that prove using it makes a difference in their code
> > with a recent version of GCC/Clang, and if it's negligible or within the
> > margin of error we'll simply ask to remove it to keep the code clean.
>
> if we want to ensure this, it can be enforced easily
> we have fate-source, that can check litterally for each av_(un)likely
> to contain a comment on the same line listing a non negligible performance
> effect. as in // branch hint increases speed by 5%

I'm generally not a fan of these hints at all, since the majority of
cases its just noise. The performance impact can vary extremely
between compilers and CPUs used, and in average its probably minimal
at best.
Even if you comment it with some speed number, it'll most of the time
be limited to one specific combination of compiler and CPU, and as
such any documented numbers are mostly meaningless.

Which is the entire problem with these likely/unlikely hints in the
first place. If you want to enforce using them in places where it
"matters", then how do you define that? One compiler on one system?
Two compiler? Two systems? Two architectures even?
You easily run into a huge potential for either endless bickering
about numbers, or a lot of questionable changes with unreproducable
"improvements" - ie. the abuse you see in every other project that 

Re: [FFmpeg-devel] [PATCH] avutil: Rename RSHIFT macro to ROUNDED_RSHIFT

2019-01-22 Thread Hendrik Leppkes
On Tue, Jan 22, 2019 at 11:58 AM Carl Eugen Hoyos  wrote:
>
> 2019-01-21 21:47 GMT+01:00, James Almer :
> > On 1/21/2019 4:09 PM, FeRD (Frank Dana) wrote:
> >> The RSHIFT macro in libavutil/common.h does not actually perform
> >> a bitwise right-shift, but rather a rounded version of the same
> >> operation, as is noted by a comment above the macro. The rounded
> >> divsion macro on the very next line is named ROUNDED_DIV, which
> >> seems far more clear.
> >>
> >> This patch renames RSHIFT to ROUNDED_RSHIFT, then updates all
> >> uses of the macro to use the new name. (These are all located
> >> in three codec files under libavcodec/.)
> >>
> >> Signed-off-by: FeRD (Frank Dana) 
> >> ---
> >>  libavcodec/mpeg4videodec.c |  4 ++--
> >>  libavcodec/vp3.c   | 16 
> >>  libavcodec/vp56.c  |  4 ++--
> >>  libavutil/common.h |  2 +-
> >>  4 files changed, 13 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/libavcodec/mpeg4videodec.c b/libavcodec/mpeg4videodec.c
> >> index f44ee76bd4..5d63ba12ba 100644
> >> --- a/libavcodec/mpeg4videodec.c
> >> +++ b/libavcodec/mpeg4videodec.c
> >> @@ -601,7 +601,7 @@ static inline int get_amv(Mpeg4DecContext *ctx, int n)
> >>  if (ctx->divx_version == 500 && ctx->divx_build == 413 && a >=
> >> s->quarter_sample)
> >>  sum = s->sprite_offset[0][n] / (1 << (a -
> >> s->quarter_sample));
> >>  else
> >> -sum = RSHIFT(s->sprite_offset[0][n] * (1 <<
> >> s->quarter_sample), a);
> >> +sum = ROUNDED_RSHIFT(s->sprite_offset[0][n] * (1 <<
> >> s->quarter_sample), a);
> >>  } else {
> >>  dx= s->sprite_delta[n][0];
> >>  dy= s->sprite_delta[n][1];
> >> @@ -623,7 +623,7 @@ static inline int get_amv(Mpeg4DecContext *ctx, int n)
> >>  v   += dx;
> >>  }
> >>  }
> >> -sum = RSHIFT(sum, a + 8 - s->quarter_sample);
> >> +sum = ROUNDED_RSHIFT(sum, a + 8 - s->quarter_sample);
> >>  }
> >>
> >>  if (sum < -len)
> >> diff --git a/libavcodec/vp3.c b/libavcodec/vp3.c
> >> index a5d8c2ed0b..13b3d6e22a 100644
> >> --- a/libavcodec/vp3.c
> >> +++ b/libavcodec/vp3.c
> >> @@ -861,10 +861,10 @@ static int unpack_vectors(Vp3DecodeContext *s,
> >> GetBitContext *gb)
> >>
> >>  if (s->chroma_y_shift) {
> >>  if (s->macroblock_coding[current_macroblock] ==
> >> MODE_INTER_FOURMV) {
> >> -motion_x[0] = RSHIFT(motion_x[0] + motion_x[1] +
> >> - motion_x[2] + motion_x[3],
> >> 2);
> >> -motion_y[0] = RSHIFT(motion_y[0] + motion_y[1] +
> >> - motion_y[2] + motion_y[3],
> >> 2);
> >> +motion_x[0] = ROUNDED_RSHIFT(motion_x[0] +
> >> motion_x[1] +
> >> + motion_x[2] +
> >> motion_x[3], 2);
> >> +motion_y[0] = ROUNDED_RSHIFT(motion_y[0] +
> >> motion_y[1] +
> >> + motion_y[2] +
> >> motion_y[3], 2);
> >>  }
> >>  motion_x[0] = (motion_x[0] >> 1) | (motion_x[0] & 1);
> >>  motion_y[0] = (motion_y[0] >> 1) | (motion_y[0] & 1);
> >> @@ -873,10 +873,10 @@ static int unpack_vectors(Vp3DecodeContext *s,
> >> GetBitContext *gb)
> >>  s->motion_val[1][frag][1] = motion_y[0];
> >>  } else if (s->chroma_x_shift) {
> >>  if (s->macroblock_coding[current_macroblock] ==
> >> MODE_INTER_FOURMV) {
> >> -motion_x[0] = RSHIFT(motion_x[0] + motion_x[1],
> >> 1);
> >> -motion_y[0] = RSHIFT(motion_y[0] + motion_y[1],
> >> 1);
> >> -motion_x[1] = RSHIFT(motion_x[2] + motion_x[3],
> >> 1);
> >> -motion_y[1] = RSHIFT(motion_y[2] + motion_y[3],
> >> 1);
> >> +motion_x[0] = ROUNDED_RSHIFT(motion_x[0] +
> >> motion_x[1], 1);
> >> +motion_y[0] = ROUNDED_RSHIFT(motion_y[0] +
> >> motion_y[1], 1);
> >> +motion_x[1] = ROUNDED_RSHIFT(motion_x[2] +
> >> motion_x[3], 1);
> >> +motion_y[1] = ROUNDED_RSHIFT(motion_y[2] +
> >> motion_y[3], 1);
> >>  } else {
> >>  motion_x[1] = motion_x[0];
> >>  motion_y[1] = motion_y[0];
> >> diff --git a/libavcodec/vp56.c b/libavcodec/vp56.c
> >> index b69fe6c176..9359b48bc6 100644
> >> --- a/libavcodec/vp56.c
> >> +++ b/libavcodec/vp56.c
> >> @@ -197,8 +197,8 @@ static void vp56_decode_4mv(VP56Context *s, int row,
> >> int col)
> >>
> >>  /* chroma vectors are average luma vectors */
> >>  if (s->avctx->codec->id == AV_CODEC_ID_VP5) {
> >> -s->mv[4].x = s->mv[5].x = RSHIFT(mv.x,2);
> >> -s->mv[4].y = s->mv[5].y = 

Re: [FFmpeg-devel] [FFmpeg-cvslog] avutil/mem: Optimize fill32() by unrolling and using 64bit

2019-01-20 Thread Hendrik Leppkes
On Sun, Jan 20, 2019 at 10:38 PM Carl Eugen Hoyos  wrote:
>
> 2019-01-20 22:22 GMT+01:00, Michael Niedermayer :
> > ffmpeg | branch: master | Michael Niedermayer  | Thu
> > Jan 17 22:35:10 2019 +0100| [12b1338be376a3e5fb606d9fe41b58dc4a9e62c7] |
> > committer: Michael Niedermayer
> >
> > avutil/mem: Optimize fill32() by unrolling and using 64bit
> >
> > Reviewed-by: Marton Balint 
> > Signed-off-by: Michael Niedermayer 
> >
> >> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=12b1338be376a3e5fb606d9fe41b58dc4a9e62c7
> > ---
> >
> >  libavutil/mem.c | 12 
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/libavutil/mem.c b/libavutil/mem.c
> > index 6149755a6b..88fe09b179 100644
> > --- a/libavutil/mem.c
> > +++ b/libavutil/mem.c
> > @@ -399,6 +399,18 @@ static void fill32(uint8_t *dst, int len)
> >  {
> >  uint32_t v = AV_RN32(dst - 4);
> >
> > +#if HAVE_FAST_64BIT
>
> I suspect this should be !X86_32
>

fast_64bit is set on any native 64-bit platform. If you can prove that
its faster on some 32-bit platforms as well, numbers shall be
required.

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


Re: [FFmpeg-devel] [PATCHv2 2/6] avcodec/vp3dsp: add 12 pixel loop filter functions

2019-01-14 Thread Hendrik Leppkes
On Mon, Jan 14, 2019 at 9:48 PM Peter Ross  wrote:
>
> ---
>  libavcodec/vp3dsp.c | 28 
>  libavcodec/vp3dsp.h |  5 +
>  2 files changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/libavcodec/vp3dsp.c b/libavcodec/vp3dsp.c
> index 4e08ee0b8f..de0130a9cf 100644
> --- a/libavcodec/vp3dsp.c
> +++ b/libavcodec/vp3dsp.c
> @@ -228,14 +228,14 @@ static void vp3_idct_dc_add_c(uint8_t *dest /* align 8 
> */, ptrdiff_t stride,
>  block[0] = 0;
>  }
>
> -static void vp3_v_loop_filter_c(uint8_t *first_pixel, ptrdiff_t stride,
> -int *bounding_values)
> +static av_always_inline void vp3_v_loop_filter_c(uint8_t *first_pixel, 
> ptrdiff_t stride,
> + int *bounding_values, int 
> count)
>  {
>  unsigned char *end;
>  int filter_value;
>  const ptrdiff_t nstride = -stride;
>
> -for (end = first_pixel + 8; first_pixel < end; first_pixel++) {
> +for (end = first_pixel + count; first_pixel < end; first_pixel++) {
>  filter_value = (first_pixel[2 * nstride] - first_pixel[stride]) +
> (first_pixel[0] - first_pixel[nstride]) * 3;
>  filter_value = bounding_values[(filter_value + 4) >> 3];
> @@ -245,13 +245,13 @@ static void vp3_v_loop_filter_c(uint8_t *first_pixel, 
> ptrdiff_t stride,
>  }
>  }
>
> -static void vp3_h_loop_filter_c(uint8_t *first_pixel, ptrdiff_t stride,
> -int *bounding_values)
> +static av_always_inline void vp3_h_loop_filter_c(uint8_t *first_pixel, 
> ptrdiff_t stride,
> + int *bounding_values, int 
> count)
>  {
>  unsigned char *end;
>  int filter_value;
>
> -for (end = first_pixel + 8 * stride; first_pixel != end; first_pixel += 
> stride) {
> +for (end = first_pixel + count * stride; first_pixel != end; first_pixel 
> += stride) {
>  filter_value = (first_pixel[-2] - first_pixel[1]) +
> (first_pixel[ 0] - first_pixel[-1]) * 3;
>  filter_value = bounding_values[(filter_value + 4) >> 3];
> @@ -261,6 +261,18 @@ static void vp3_h_loop_filter_c(uint8_t *first_pixel, 
> ptrdiff_t stride,
>  }
>  }
>
> +#define LOOP_FILTER(prefix, suffix, dim, count) \
> +void prefix##_##dim##_loop_filter_##count##suffix(uint8_t *first_pixel, 
> ptrdiff_t stride, \
> +int *bounding_values) \
> +{ \
> +vp3_##dim##_loop_filter_c(first_pixel, stride, bounding_values, count); \
> +}
> +
> +static LOOP_FILTER(vp3,_c, v, 8)
> +static LOOP_FILTER(vp3,_c, h, 8)
> +LOOP_FILTER(ff_vp3dsp, , v, 12)
> +LOOP_FILTER(ff_vp3dsp, , h, 12)
> +
>  static void put_no_rnd_pixels_l2(uint8_t *dst, const uint8_t *src1,
>   const uint8_t *src2, ptrdiff_t stride, int 
> h)
>  {
> @@ -285,8 +297,8 @@ av_cold void ff_vp3dsp_init(VP3DSPContext *c, int flags)
>  c->idct_put  = vp3_idct_put_c;
>  c->idct_add  = vp3_idct_add_c;
>  c->idct_dc_add   = vp3_idct_dc_add_c;
> -c->v_loop_filter = vp3_v_loop_filter_c;
> -c->h_loop_filter = vp3_h_loop_filter_c;
> +c->v_loop_filter = vp3_v_loop_filter_8_c;
> +c->h_loop_filter = vp3_h_loop_filter_8_c;
>
>  if (ARCH_ARM)
>  ff_vp3dsp_init_arm(c, flags);
> diff --git a/libavcodec/vp3dsp.h b/libavcodec/vp3dsp.h
> index f55a7f834f..30a76100d9 100644
> --- a/libavcodec/vp3dsp.h
> +++ b/libavcodec/vp3dsp.h
> @@ -43,8 +43,13 @@ typedef struct VP3DSPContext {
>  void (*idct_dc_add)(uint8_t *dest, ptrdiff_t stride, int16_t *block);
>  void (*v_loop_filter)(uint8_t *src, ptrdiff_t stride, int 
> *bounding_values);
>  void (*h_loop_filter)(uint8_t *src, ptrdiff_t stride, int 
> *bounding_values);
> +void (*v_loop_filter_12)(uint8_t *src, ptrdiff_t stride, int 
> *bounding_values);
> +void (*h_loop_filter_12)(uint8_t *src, ptrdiff_t stride, int 
> *bounding_values);
>  } VP3DSPContext;
>
> +void ff_vp3dsp_v_loop_filter_12(uint8_t *first_pixel, ptrdiff_t stride, int 
> *bounding_values);
> +void ff_vp3dsp_h_loop_filter_12(uint8_t *first_pixel, ptrdiff_t stride, int 
> *bounding_values);
> +

It seems weird to have DSP member methods for the 12 pixel loop
filter, but then never assign or use them. Is that a remnant from an
earlier approach?
For consistency, it would seem the most logical to add the new loop
filter implementation to the DSP context as well, just like the old
one. Or at the very least not introduce method variables there that
remain NULL and unused.

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


Re: [FFmpeg-devel] [PATCH V2 1/2] lavf/utils: fix error like "offset 0x1f85: partial file"

2019-01-14 Thread Hendrik Leppkes
On Mon, Jan 14, 2019 at 5:32 PM Hendrik Leppkes  wrote:
>
> On Mon, Jan 14, 2019 at 5:09 PM Jun Zhao  wrote:
> >
> > fix the issue like "offset 0x1f85: partial file" when demuxer mp4
> > from http/https/crypto
> >
> > Signed-off-by: Jun Zhao 
> > ---
> >  libavformat/utils.c |3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/libavformat/utils.c b/libavformat/utils.c
> > index 7afef54..92a0eb7 100644
> > --- a/libavformat/utils.c
> > +++ b/libavformat/utils.c
> > @@ -2103,7 +2103,8 @@ void ff_configure_buffers_for_index(AVFormatContext 
> > *s, int64_t time_tolerance)
> > "optimally without knowing the protocol\n");
> >  }
> >
> > -if (proto && !(strcmp(proto, "file") && strcmp(proto, "pipe") && 
> > strcmp(proto, "cache")))
> > +if (proto && !(strcmp(proto, "file") && strcmp(proto, "pipe") && 
> > strcmp(proto, "cache") &&
> > +   strcmp(proto, "https") && strcmp(proto, "http") && 
> > strcmp(proto, "crypto")))
> >  return;
> >
> >  for (ist1 = 0; ist1 < s->nb_streams; ist1++) {
>
> Isn't the entire point of this function to do stuff for network
> streams,  like http?
> If there is a bug in there, it should probably be fixed, and not just 
> disabled?
>

Apparently mov.c is also the only caller of this function, and http is
probably the only network protocol you're ever going to read a mov/mp4
file from, so if its disabled for those, might as well delete it. So
that should be given some careful thought.

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


Re: [FFmpeg-devel] [PATCH V2 1/2] lavf/utils: fix error like "offset 0x1f85: partial file"

2019-01-14 Thread Hendrik Leppkes
On Mon, Jan 14, 2019 at 5:09 PM Jun Zhao  wrote:
>
> fix the issue like "offset 0x1f85: partial file" when demuxer mp4
> from http/https/crypto
>
> Signed-off-by: Jun Zhao 
> ---
>  libavformat/utils.c |3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index 7afef54..92a0eb7 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -2103,7 +2103,8 @@ void ff_configure_buffers_for_index(AVFormatContext *s, 
> int64_t time_tolerance)
> "optimally without knowing the protocol\n");
>  }
>
> -if (proto && !(strcmp(proto, "file") && strcmp(proto, "pipe") && 
> strcmp(proto, "cache")))
> +if (proto && !(strcmp(proto, "file") && strcmp(proto, "pipe") && 
> strcmp(proto, "cache") &&
> +   strcmp(proto, "https") && strcmp(proto, "http") && 
> strcmp(proto, "crypto")))
>  return;
>
>  for (ist1 = 0; ist1 < s->nb_streams; ist1++) {

Isn't the entire point of this function to do stuff for network
streams,  like http?
If there is a bug in there, it should probably be fixed, and not just disabled?

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


Re: [FFmpeg-devel] [PATCH] doc/developer: require transparency about sponshorships.

2019-01-13 Thread Hendrik Leppkes
On Sun, Jan 13, 2019 at 2:40 PM Nicolas George  wrote:
>
> James Almer (12019-01-13):
> > I seem to remember the famous votes count voices, if one were to be called.
>
> You should check again, the rules state that mails without arguments do
> not count.
>
> > Nicolas, no one is in favor of this thing. It's an invasion of privacy
>
> I do not consider this specific point worthy of privacy protection.
>

You can't ask for arguments then dismiss the ones you are given based
on your opinions.
I consider my finances and employment my own business, and will never
disclose it on *public* mailing list.

Therefor, such a policy is entirely unacceptable.

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


Re: [FFmpeg-devel] [PATCH] doc/developer: require transparency about sponshorships.

2019-01-11 Thread Hendrik Leppkes
On Fri, Jan 11, 2019 at 8:05 PM Nicolas George  wrote:
>
> On the other hand, I have observed in the past patches that were of poor
> quality and suspected they were the result of sponsorships. I would like
> to know. Would you not?
>

Wanting to know and forcing everyone to tell you are two entirely
different things.
Its everyones right to keep their finances private. Would I be forced
to disclose my hourly wages and then determine how long I worked on a
patch, just because I did it during my day job? Thats not going to
happen.

To take a line from your post:
Are you against privacy?

Patches should generally be considered on their own merit. Any such
information will only bias any discussions and result in more
conflict.

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


Re: [FFmpeg-devel] [PATCH] checkasm/af_afir: relax the max allowed absolute difference

2019-01-11 Thread Hendrik Leppkes
On Fri, Jan 11, 2019 at 2:12 PM Derek Buitenhuis
 wrote:
>
> On 10/01/2019 23:34, James Almer wrote:
> > -if (!float_near_abs_eps(cdst[i], odst[i], FLT_EPSILON)) {
> > +if (!float_near_abs_eps(cdst[i], odst[i], 6.2e-05)) {
>
> Can you elaborate a bit more on this? FLT_EPSILON is used correctly here
> as far as I can tell, and it is not clear why it fails on x86_32, and why
> we should choose an arbitrary unportable number instead (who knows if it
> explodes on weird systems).
>

Because the computation accumulates more inaccuarcy then FLT_EPSILON
allows for. That value is really not of that great use. If you have
two accurate numbers and do one calculation, it may work, but if you
do a whole bunch of them, the error accumulates and eventually gets
bigger then FLT_EPSILON.
x86_32 floating point is for $reasons a tad bit less accurate then on
x86_64, for example, resulting in the test failing. We have some other
float tests that  do (or used to) fail sporadically due to inaccuracy
problems, which sometimes where fixed by similar means - or
multiplifying FLT_EPSILON to make it bigger.

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


Re: [FFmpeg-devel] [PATCH] lavc/qsvenc: set BRCParamMultiplier to aviod BRC overflow

2019-01-11 Thread Hendrik Leppkes
On Fri, Jan 11, 2019 at 1:18 PM Carl Eugen Hoyos  wrote:
>
> 2019-01-11 13:09 GMT+01:00, Zhong Li :
> > +//libmfx BRC parameters are 16 bits thus maybe overflow, then
> > BRCParamMultiplier is needed
> > +target_bitrate_kbps = avctx->bit_rate / 1000;
> > +max_bitrate_kbps = avctx->rc_max_rate / 1000;
> > +brc_param_multiplier = (FFMAX(target_bitrate_kbps, max_bitrate_kbps) +
> > 0x1) / 0x1;
>
> How will this behave if the user sets 100MBit?
>

Its really quite simple math. It'll cause the multipler to become 2,
effectively dividing all numbers by two, so instead of trying to pass
102400 kbps bitrate, it'll pass 51200 * 2 kbps bitrate, which fits
into the 16-bit value.

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


Re: [FFmpeg-devel] Memory leaks using x265 encoder

2019-01-02 Thread Hendrik Leppkes
On Wed, Jan 2, 2019 at 4:33 PM Oliver Collyer  wrote:
>
> Can you clarify what you mean by "static variables are unacceptable". They're 
> all over the place in ffmpeg (random example 
> https://github.com/FFmpeg/FFmpeg/blob/a0ac49e38ee1d1011c394d7be67d0f08b2281526/libavcodec/ffjni.c
>  
> )...
>  what am I missing?
>

We try to clean them up where we can, and new ones are unacceptable.

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


Re: [FFmpeg-devel] [PATCH]configure: ole32 is needed for shared libavcodec linking with dxva2.

2018-12-30 Thread Hendrik Leppkes
On Sun, Dec 30, 2018 at 9:16 PM Carl Eugen Hoyos  wrote:
>
> Hi!
>
> Attached patch fixes a Windows linking issue described in ticket #7642,
> only shared linking was tested so far.
>

ole32 is a dependency of dxva2 already, it should already be pulled in
through that, shouldn't it?

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


  1   2   3   4   5   6   7   8   9   10   >