Re: [FFmpeg-devel] [PATCH] hwcontext_d3d11: Log adapter details on device creation

[Jan Ekstrom]

On Tue, Nov 14, 2017 at 4:05 PM, Mark Thompson  wrote:
> This is helpful to know what device has actually been used.
> ---

Change looks alright, and especially in multi-GPU contexts like
laptops this can be quite useful - as you would learn which device you
have just tried to utilize.

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

Re: [FFmpeg-devel] [PATCH] hwcontext_d3d11va: properly reset values after release/close

[Jan Ekstrom]

On Nov 24, 2017 03:01, "Jan Ekström"  wrote:

Makes the uninit function re-entrable, which can be a common case
when an API user first tries to initialize its context, fails, and
then finally unrefs the AVHWDevice.

Fixes a crash reported by sm2345 on IRC


Relevant backtrace I received from the user:

https://pastebin.com/MtWTEgmc

Regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] configure: add audio_frame_queue dependency for aptx codec

[Jan Ekstrom]

On Sun, Nov 19, 2017 at 4:01 PM, James Darnley  wrote:
> ---
>  configure | 2 ++
>  1 file changed, 2 insertions(+)
>

LGTM, fixes shared linkage reported on IRC.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] On in-tree external headers

[Jan Ekstrom]

On Sun, Nov 5, 2017 at 10:40 PM, Timo Rothenpieler
 wrote:
> For once, there should be a good reason to do so.
>

Agreed.

> In case of nvidia the headers in this form is otherwise unobtainable, and
> it's also partially modified specifically for use in ffmpeg.
> Getting the original headers is also not straight forward as you need an
> nvidia developer account, which you cannot just register for, but you need
> to apply for.
>

Just my 20 cents, but this point is kind of less straightforward.
Sure, we like having more features and various vendors also like the
fact that we've made their things easily usable. But this also leads
to the in my honest opinion uncomfortable case where due to the first
vendor being an unfortunate REDACTED regarding the SDK, anything that
comes afterwards and handles itself properly seems to be getting less
nice handling if requested to behave like a normal thing in the
ecosystem. If we think about a case where the other vendor's SDK would
have been presented here first, and we would have made it available
with a dependency on its SDK, it would have been much less likely that
we would have added this latter feature with the headers there. But
this is just theoretical at this point since no proper SDK has come up
on either vendor's side. I think VA-API is currently the only thing
with a "proper SDK" (and the other Intel thing is one where someone
took the time to make it into a proper "SDK").

To be honest, I wouldn't be against removing nvidia's headers and
having them separate just to have the result be similar (even though
users would most definitely be confused at first). Avisynth/Avxsynth
is kind of a special kind of open source mess where having the header
inside makes sense (and let's not even go into Avisynth+ which doesn't
seem to want to be binary compatible with Avisynth 2.6).

> I also feel like whatever this rule would look like, it's already practiced
> that way. There isn't really a way not do decide this on a case by case
> basis. Luckily it's not something that comes up every other day.
> If someone would submit random third party library headers to compat/ for no
> apparent reason other than comfort, it would certainly be rejected.

Yes, the rule most certainly would contain something along the lines
of "...this is something that should be considered on a case-by-case
basis with a reasoning being mentioned and considered..."

Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] On in-tree external headers

[Jan Ekstrom]

On Mon, Oct 30, 2017 at 9:51 PM, Mark Thompson  wrote:
> PPS:
>
> The position stated above would imply removing the avisynth headers.  Can 
> anyone who uses it comment on what would be required for that?

Avisynth headers are available from the Avisynth SDK that comes with
each installer of Avisynth. The reason why some projects made their
own forks of the header is because of the useful features added during
the (rather long) development phase of the 2.6 release, during which
the GPL exceptions regarding the header were removed. And thus people
re-implemented them basing on the 2.5.8 stable header. Not sure if the
GPL exceptions were re-instated with the final release of Avisynth
2.6, and what is the status with the headers that come with the
Avisynth+ project. In any case, the headers are readily available from
various places.

Otherwise the only difference with the included header would be
Avxsynth compatibility? Which is quite a bit less used than original
Avisynth on wine, or Vapoursynth natively on *nix at this point. It
was one of those things where a company forked an old version of a
project and made it compile on *nix, while most of the plugins etc
were quite Windows-specific. The project still does exist and is
available, though.


Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] lavf/avio: temporarily accept 0 as EOF.

[Jan Ekstrom]

On Fri, Oct 27, 2017 at 9:56 PM, Jan Ekstrom <jee...@gmail.com> wrote:
> On Fri, Oct 27, 2017 at 9:46 PM, Nicolas George <geo...@nsup.org> wrote:
>> Print a warning to let applicatios fix their use.
>> After a deprecation period, check with a low-level assert.
>> Also make the constraint explicit in the doxygen comment.
>
> Difference to the previous version:
>> -av_log(s, AV_LOG_WARNING, "Invalid return value 0 for stream 
>> protocol\n");
>> +av_log(NULL, AV_LOG_WARNING, "Invalid return value 0 for stream 
>> protocol\n");
>
> Thus LGTM.
>
> Jan

The patch set has been applied.


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

Re: [FFmpeg-devel] [PATCH 1/3] examples/avio_reading: return AVERROR_EOF at EOF.

[Jan Ekstrom]

On Fri, Oct 27, 2017 at 9:46 PM, Nicolas George  wrote:
> Signed-off-by: Nicolas George 

LGTM


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

Re: [FFmpeg-devel] [PATCH 3/3] lavf/aviobuf: return EINVAL when reading from a write-only context.

[Jan Ekstrom]

On Fri, Oct 27, 2017 at 9:46 PM, Nicolas George  wrote:
> Signed-off-by: Nicolas George 
> ---

LGTM

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

Re: [FFmpeg-devel] [PATCH 2/3] lavf/avio: temporarily accept 0 as EOF.

[Jan Ekstrom]

On Fri, Oct 27, 2017 at 9:46 PM, Nicolas George  wrote:
> Print a warning to let applicatios fix their use.
> After a deprecation period, check with a low-level assert.
> Also make the constraint explicit in the doxygen comment.

Difference to the previous version:
> -av_log(s, AV_LOG_WARNING, "Invalid return value 0 for stream 
> protocol\n");
> +av_log(NULL, AV_LOG_WARNING, "Invalid return value 0 for stream 
> protocol\n");

Thus LGTM.

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

Re: [FFmpeg-devel] [PATCH 2/3] lavf/avio: temporarily accept 0 as EOF.

[Jan Ekstrom]

On Fri, Oct 27, 2017 at 9:33 PM, Nicolas George  wrote:
> It is standard parlance in networking: stream protocols produce a stream
> of octets, without any additional structure, while packet protocols
> produce packets, which are delimited at protocol level and visible by
> the application, and can be empty.
>

Gotcha. Seems like it's called "/* default: stream file */" in the
current code base, which is why I couldn't find references to stream
protocols.

> FFmpeg distinguishes packet protocols with the max_packet_size: if it is
> 0, it is a stream protocol, if not a packet protocol.

Gotcha^2.

Please send the patch-set v2 with the related fixes (the av_log in 2/3
and the correct ret in 3/3) as with that I think this all looks good
to go :) .

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

Re: [FFmpeg-devel] [PATCH 2/3] lavf/avio: temporarily accept 0 as EOF.

[Jan Ekstrom]

On Wed, Oct 25, 2017 at 11:22 AM, Nicolas George  wrote:
> +static int read_packet_wrapper(AVIOContext *s, uint8_t *buf, int size)
> +{
> +int ret;
> +
> +if (!s->read_packet)
> +return AVERROR_EOF;
> +ret = s->read_packet(s->opaque, buf, size);
> +#if FF_API_OLD_AVIO_EOF_0
> +if (!ret && !s->max_packet_size) {
> +av_log(s, AV_LOG_WARNING, "Invalid return value 0 for stream 
> protocol\n");
> +ret = AVERROR_EOF;
> +}
> +#else
> +av_assert2(ret || s->max_packet_size);
> +#endif
> +return ret;
> +}
> +

Built and tested locally and the effect seems to be the one wished,
although it seems like it is complaining about the AVClass being
nullptr? Is this something inherent to custom AVIO contexts and it's
the client that's supposed to fix this? Log follows:

> [ffmpeg] av_log callback called with bad parameters (NULL AVClass).
> [ffmpeg] This is a bug in one of Libav/FFmpeg libraries used.
> [ffmpeg] Invalid return value 0 for stream protocol


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

Re: [FFmpeg-devel] [PATCH 2/3] lavf/avio: temporarily accept 0 as EOF.

[Jan Ekstrom]

On Wed, Oct 25, 2017 at 11:22 AM, Nicolas George  wrote:
> Print a warning to let applicatios fix their use.
> After a deprecation period, check with a low-level assert.
> Also make the constraint explicit in the doxygen comment.
>
> Signed-off-by: Nicolas George 

Thanks for the patch.

> ---
>  libavformat/avio.h|  2 ++
>  libavformat/aviobuf.c | 30 +-
>  libavformat/version.h |  3 +++
>  3 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/libavformat/avio.h b/libavformat/avio.h
> index 19ecd96eb7..76ff7cd81e 100644
> --- a/libavformat/avio.h
> +++ b/libavformat/avio.h
> @@ -452,6 +452,8 @@ void avio_free_directory_entry(AVIODirEntry **entry);
>   * @param write_flag Set to 1 if the buffer should be writable, 0 otherwise.
>   * @param opaque An opaque pointer to user-specific data.
>   * @param read_packet  A function for refilling the buffer, may be NULL.
> + * For stream protocols, must never return 0 but rather
> + * a proper AVERROR code.

Otherwise OK, but since this is the first mention of "stream protocol"
in the repo and el goog gives me just "Internet Stream Protocol" so I
would like an explanation of what this means and how it connects to
"custom AVIO implementations/wrappers"?

>   * @param write_packet A function for writing the buffer contents, may be 
> NULL.
>   *The function may not change the input buffers content.
>   * @param seek A function for seeking to specified byte position, may be 
> NULL.
> diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
> index 3e9d774a13..bb5bcf7a14 100644
> --- a/libavformat/aviobuf.c
> +++ b/libavformat/aviobuf.c
> @@ -524,6 +524,24 @@ void avio_write_marker(AVIOContext *s, int64_t time, 
> enum AVIODataMarkerType typ
>  s->last_time = time;
>  }
>
> +static int read_packet_wrapper(AVIOContext *s, uint8_t *buf, int size)
> +{
> +int ret;
> +
> +if (!s->read_packet)
> +return AVERROR_EOF;
> +ret = s->read_packet(s->opaque, buf, size);
> +#if FF_API_OLD_AVIO_EOF_0
> +if (!ret && !s->max_packet_size) {

Can you explain how you can utilize max_packet_size as a note
regarding if zero is EOF or not? Is it just a variable that happens to
be set to zero with custom AVIO contexts, or is there some other logic
behind this?

> +av_log(s, AV_LOG_WARNING, "Invalid return value 0 for stream 
> protocol\n");
> +ret = AVERROR_EOF;
> +}
> +#else
> +av_assert2(ret || s->max_packet_size);
> +#endif
> +return ret;
> +}
> +
>  /* Input stream */
>
>  static void fill_buffer(AVIOContext *s)
> @@ -562,10 +580,7 @@ static void fill_buffer(AVIOContext *s)
>  len = s->orig_buffer_size;
>  }
>
> -if (s->read_packet)
> -len = s->read_packet(s->opaque, dst, len);
> -else
> -len = AVERROR_EOF;
> +len = read_packet_wrapper(s, dst, len);
>  if (len == AVERROR_EOF) {
>  /* do not modify buffer if EOF reached so that a seek back can
> be done without rereading data */
> @@ -638,10 +653,7 @@ int avio_read(AVIOContext *s, unsigned char *buf, int 
> size)
>  if (len == 0 || s->write_flag) {
>  if((s->direct || size > s->buffer_size) && !s->update_checksum) {
>  // bypass the buffer and read data directly into buf
> -if(s->read_packet)
> -len = s->read_packet(s->opaque, buf, size);
> -else
> -len = AVERROR_EOF;
> +len = read_packet_wrapper(s, buf, size);
>  if (len == AVERROR_EOF) {
>  /* do not modify buffer if EOF reached so that a seek 
> back can
>  be done without rereading data */
> @@ -708,7 +720,7 @@ int avio_read_partial(AVIOContext *s, unsigned char *buf, 
> int size)
>  return -1;
>
>  if (s->read_packet && s->write_flag) {
> -len = s->read_packet(s->opaque, buf, size);
> +len = read_packet_wrapper(s, buf, size);
>  if (len > 0)
>  s->pos += len;
>  return len;
> diff --git a/libavformat/version.h b/libavformat/version.h
> index 0feb788c36..358ab91ab2 100644
> --- a/libavformat/version.h
> +++ b/libavformat/version.h
> @@ -79,6 +79,9 @@
>  #ifndef FF_API_OLD_ROTATE_API
>  #define FF_API_OLD_ROTATE_API   (LIBAVFORMAT_VERSION_MAJOR < 59)
>  #endif
> +#ifndef FF_API_OLD_AVIO_EOF_0
> +#define FF_API_OLD_AVIO_EOF_0   (LIBAVFORMAT_VERSION_MAJOR < 59)
> +#endif
>
>
>  #ifndef FF_API_R_FRAME_RATE
> --
> 2.14.2

Otherwise LGTM. Will test in a moment.


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

Re: [FFmpeg-devel] [PATCH] libavformat: not treat 0 as EOF

[Jan Ekstrom]

On Tue, Oct 24, 2017 at 10:43 PM, James Almer  wrote:
> Maybe first ask what the workaround Nicolas mentioned is about?

This was not meant to push anything on anyone. Just wanted to let him
know what had been come up with elsewhere.

> And preferably don't quote me on this subject. This is not my area of
> expertise and i simply gave a suggestion to keep the old behavior at
> least for a while, assuming this is an API break. And my suggestion was
> in fact the opposite of what everyone else agreed on in the end.

Sorry. It's just that I finally got people talking about it, which as
you can see by this thread is not something happening as much here.
Will not do again. And yes, but you seemed to agree at the end. That
might have just been my misunderstanding.

And yes, I am not an expert on this either. I just knew about the
general breakage and the fact that no actions were yet taken regarding
it, so I tried to start a discussion about alternatives of handling
this.

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

Re: [FFmpeg-devel] [PATCH] libavformat: not treat 0 as EOF

[Jan Ekstrom]

On Tue, Oct 24, 2017 at 10:17 PM, Nicolas George <geo...@nsup.org> wrote:
> Le tridi 3 brumaire, an CCXXVI, Jan Ekstrom a écrit :
> I do not consider this a public API change because 0 was never
> documented as a valid return value for a read_packet callback, while
> AVERROR_EOF has been around for a long time.
>

You might be very true, but funny enough I just checked the AVIO
example doc/examples/avio_reading.c, and the read function there does
not seem to utilize AVERROR_EOF either :) .

> But my goal is not to break existing code on purpose. Fortunately, there
> is a rather simple workaround, I will try to implement it tomorrow.
> Still, lacking a simple test case, I will not be able to test it.
>

Just for your information, I decided to call people out a bit on the
IRC channel to see what people would prefer, and it seems like James,
BtbN and Martin voiced their opinion that they would like a flag that
would let you enable the 0 != EOF behavior. Additionally, a warning
would be given out if zero is returned and the flag is unset. Quote
follows:

> <+JEEB> can anyone else chime on the EOF thing, if we want to
> either A) leave things as they are and just break old
> clients with an API behavior change B) add an option for
> the new behavior , or C) revert the 0 != EOF thing
> <+JEEB> because while those people that are on IRC have most
> likely adapted their stuff, we will have quite a bit of
> breakage due to this :P
> <+JEEB> I'd probably go for B but I'm not sure how many places I'd
> have to poke to add the option
> < jamrial> maybe a temporary option to recover the old behavior
>should be added for the usual period of ~2 years, and a
>notice about it being removed eventually added
> < jamrial> basically, deprecating the old behavior and leaving a
>compat mode for it
> <+JEEB> yes
> < jamrial> library users will nontheless have to update their code
>to enable said option
> < jamrial> then again to remove it two years from now :p
> < jamrial> so dunno
> < jamrial> is this change, unintended breakage aside, a good one?
>because if it doesn't really bring any worthwile
>benefit maybe we should just revert it
> <+JEEB> the requirement for the change was zero-byte UDP packets
> which seem to be valid. originally the code was added into
> the UDP lavf module, but then it was requested to be moved
> into lavf general
> <+JEEB> which thus broke the API behavior
> <@nevcairiel> jamrial: i have always questioned the usefulness of
>   the change, but apparently there is obscure  crazy
>   things that might use it
> <+JEEB> yes, most users don't find that stuff on their networks
> <@nevcairiel> and re: flag, if anything it should be inverted, or
>   its still an API break =p
> <+JEEB> well flag to have the NEW behavior
> <+JEEB> so that the default behavior is the old one
> <+JEEB> that's what I meant
> < jamrial> yeah, probably
> <@BtbN> It should behave as it used to, which is imo a bit broken,
> with some flag somewhere to flip it to sane behaviour
> <+JEEB> BtbN: agreed
> <@BtbN> And it should throw a depercation warning if the flag is
> unset
> <@wbs> alternatively, only throw a warning if the flag is unset
>_and_ you return 0?
> <@wbs> then you can write code that uses AVERROR_EOF for older
>lavf versions without any ifdefs for the new flag, while
>still running without warnings on the new lavf as well

Any opinions?


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

Re: [FFmpeg-devel] [PATCH] libavformat: not treat 0 as EOF

[Jan Ekstrom]

On Tue, Oct 24, 2017 at 9:18 PM, Jan Ekstrom <jee...@gmail.com> wrote:
> This is a public API change, and I'm not sure if we were planning on
> this. I am not against the fact that zero is no longer implictly EOF,
> but this might have been worth a bit more notice as this does indeed
> break quite a few API clients as noted by nevcairiel as well. If it is
> considered that no further changes are required then I think we at the
> very least want to mention this in the change log as well as the
> doc/APIchanges file.
>

Also as another alternative we might put the new behavior under an
option. Thus those clients that want this behavior can get it, and
otherwise old clients don't just break. Then we can properly deprecate
the option and change the default then. If this makes any darn sense.
Because the final option is a revert and I don't think anyone here
wants that.


Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavformat: not treat 0 as EOF

[Jan Ekstrom]

On Tue, Oct 24, 2017 at 8:57 PM, Nicolas George  wrote:
> Le duodi 2 brumaire, an CCXXVI, James Almer a écrit :
>> https://trac.ffmpeg.org/ticket/6767
>
> I do not see in this ticket anything that allows to reproduce the issue
> without a huge amount of foreign code.
>
> Regards,

Hi,

The base idea was grasped rather quickly though, which seems to be
that all API clients utilizing their own IO seem to now require an
update to their code to return AVERROR_EOF if EOF was hit instead of
being able to return the number of read bytes, as usual.

See https://github.com/mpv-player/mpv/pull/5033/files , for example.

This is a public API change, and I'm not sure if we were planning on
this. I am not against the fact that zero is no longer implictly EOF,
but this might have been worth a bit more notice as this does indeed
break quite a few API clients as noted by nevcairiel as well. If it is
considered that no further changes are required then I think we at the
very least want to mention this in the change log as well as the
doc/APIchanges file.

Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] libavformat: not treat 0 as EOF

[Jan Ekstrom]

On Tue, Oct 17, 2017 at 11:29 AM, Daniel Kucera  wrote:
> transfer_func variable passed to retry_transfer_wrapper
> are h->prot->url_read and h->prot->url_write functions.
> These need to return EOF or other error properly.
> In case of returning >= 0, url_read/url_write is retried
> until error is returned.

This seems to have broken seeking and files ending. FLAC was
experienced yesterday but it seems like it's more general. This was
reported on the mpv users' channel by sfan5, but I feel like this
might be more general than related to just mpv. At first this was
thought to be limited to HTTP, but later was found out to be
applicable to local files as well.

Quote:
< wm4> reproduction is "play anything with lavf"
< JEEB> oh really? and seek?
< sfan5> yes
< wm4> no, seek close to end of file and see if it ends correctly. if
it freezes it's the bug
< sfan5> sounds like two different bugs
< sfan5> bug 1: files don't end correctly
< sfan5> bug 2: seeking somewhere never finishes and spins the cpu
< sfan5> 2 is what i was experiencing yesterday


Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] swscale: more accurate DITHER_COPY macro for full and limited range

[Jan Ekstrom]

On Fri, Oct 20, 2017 at 10:26 AM, Mateusz  wrote:
> W dniu 2017-10-06 o 17:33, Mateusz pisze:
>> Fixed DITHER_COPY macro (only C code), updated FATE tests.
>>
>> PSNR in tests that needed update goes from 50 to 999.99 -- the quality is 
>> there.
>
> Ping.

Hi,

The updated PSNR values look really good (and the max difference going
from 1 to 0), but unfortunately I lack the capacity to verify that the
code is doing the same thing as the original thing.

Can we please have someone's eyes on this? If a patchwork URL makes it
simpler, it's https://patchwork.ffmpeg.org/patch/5440/ .


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

Re: [FFmpeg-devel] HEVC ARM optimization

[Jan Ekstrom]

Hi,

On Fri, Oct 20, 2017 at 10:39 AM, Shengbin Meng  wrote:
> Hi,
>
> I’d like to know if anyone is dong or interested in ARM optimization for the 
> native HEVC decoder in FFmpeg?
>

Of course we are interested in optimizations. An example of how they
can be integrated is available @
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=ffbd1d2b0002576ef0d976a41ff959c635373fdc
(this is for VP9, but the idea is similar).

For HEVC we already have some of the ground work available such as
libavcodec/arm/hevcdsp_init_{arm,neon}.c , latter of which is what
sets the DSP function pointers. You can see if your existing SIMD can
fit into any of the functions that are defined by the HEVCDSPContext.


Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 4/4] lavf/movdec: request probing for an ambiguous codec tag

[Jan Ekstrom]

On Thu, Oct 19, 2017 at 10:39 AM, Rodger Combs  wrote:
> ---
>  libavformat/isom.c | 2 ++
>  1 file changed, 2 insertions(+)

This change makes sense due to the ambiguousness of the container mapping. LGTM.

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

Re: [FFmpeg-devel] [PATCH 3/4] lavf/utils: add MP2 to the probing list

[Jan Ekstrom]

On Thu, Oct 19, 2017 at 10:39 AM, Rodger Combs  wrote:
> ---

Seems like a valid change to enable the following change of enabling
probing in movdec for mp2/3 in ISOBMFF.

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

Re: [FFmpeg-devel] [PATCH] configure: add pkg-config check for alsa

[Jan Ekstrom]

> Please apply.

Got my key registered onto the system and pushed along with an update
to the commit message noting the static linking use case.

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

Re: [FFmpeg-devel] [PATCH] configure: add pkg-config check for alsa

[Jan Ekstrom]

On Mon, Oct 16, 2017 at 11:01 PM, Jan Ekström  wrote:
> ---
>  configure | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index b9a3a9bc1f..5aa642a9bb 100755
> --- a/configure
> +++ b/configure
> @@ -6254,7 +6254,8 @@ EOF
>  fi
>  check_header soundcard.h
>
> -enabled alsa && check_lib alsa alsa/asoundlib.h snd_pcm_htimestamp -lasound
> +enabled alsa && use_pkg_config alsa alsa "alsa/asoundlib.h" 
> snd_pcm_htimestamp ||
> +check_lib alsa alsa/asoundlib.h snd_pcm_htimestamp -lasound
>
>  enabled jack && check_lib jack jack/jack.h jack_client_open -ljack &&
>  check_func jack_port_get_latency_range -ljack
> --
> 2.13.6
>

Just noticed that random user on #ffmpeg had private messaged me, and
yes - this had indeed fixed his linking issues with a static ALSA
library.

Please apply.

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

Re: [FFmpeg-devel] [PATCH] configure: add libm ldflags globally

[Jan Ekstrom]

On Tue, Oct 17, 2017 at 12:38 AM, Hendrik Leppkes  wrote:
> Perhaps such libraries shouldn't hardcode -lstdc++ in there, but
> dynamically put whichever C++ library they built against in there
> instead?
> Its not like you can actually use a static library build with another
> C++ library, that *may* randomly work, but its not something anyone
> should be doing.
>
> - Hendrik

Yup,

The project should know which C++ runtime it is built against and put
that into the .pc file's Libs.private. If the project doesn't have an
idea on how to automate this, a solution like the one utilized by zimg
is valid as well:

* Check for an environment variable during configuration and set the
default to `-lstdc++`
(https://github.com/sekrit-twc/zimg/blob/ae673e02c8e934915e05daeb120c7cb1b500a0a4/configure.ac#L55..L56)
* Set that value in your pkg-config template
(https://github.com/sekrit-twc/zimg/blob/ae673e02c8e934915e05daeb120c7cb1b500a0a4/zimg.pc.in#L13)

This way in case the value is incorrect the user can set the
environment variable, and then get what is needed.


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

Re: [FFmpeg-devel] [PATCH] configure: add libm ldflags globally

[Jan Ekstrom]

On Mon, Oct 16, 2017 at 7:31 PM, James Almer  wrote:
> On 10/14/2017 12:59 PM, James Almer wrote:
>> It's used by every library, and by making it global we simplify a lot
>> of checks.
>>

LGTM.

I dislike the fact that we have to fix issues caused by 3rd party
libraries' pkg-config files but at this point the simplification for
things we also utilize kind of makes sense.

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

Re: [FFmpeg-devel] [PATCH] lavf/dashenc: add format_options option, mirroring segment.c

[Jan Ekstrom]

On Tue, Sep 26, 2017 at 4:52 AM, Rodger Combs  wrote:
> ---
>  libavformat/dashenc.c | 13 +
>  1 file changed, 13 insertions(+)

LGTM.

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

Re: [FFmpeg-devel] [PATCH] GnuTLS: eat PREMATURE_TERMINATION error

[Jan Ekstrom]

Hi,

On Fri, Sep 15, 2017 at 11:04 AM, Tatsuyuki Ishi
 wrote:
> Subject: [PATCH] GnuTLS: eat PREMATURE_TERMINATION error
>
> GnuTLS is too strict on the SSL shutdown alert, and it's neither
> mandatory in the spec or critical. As it's ignored in OpenSSL, we
> should also suppress it in GnuTLS as well.
>

If anyone wants to test this, Youtube is a good example of this
message getting spammed. If we ignore this with OpenSSL as well, then
I totally agree with this change.


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

Re: [FFmpeg-devel] Switching of automatically detected libraries [v2]

[Jan Ekstrom]

Hi,

On Wed, Aug 30, 2017 at 3:08 PM, Clément Bœsch  wrote:
> ...
> Thanks to people who review the previous patchset and tested it. If
> someone wants to test without messing with patches, the changes are
> available on github/ubitux/FFmpeg#autodetect.
>

Just got to testing this patch set, and it seems to be working nicely.
Definitely a step to the right direction in my opinion by at least
being able to not have a bunch of things autodetect. +1 from me.

Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Add new MPEG bitstream filter

[Jan Ekstrom]

Hi,

On Tue, Aug 1, 2017 at 3:22 PM, David Griffiths
 wrote:
> Given an MPEG1/2 stream, this bitstream filter can be used to modify
> the sequence headers. The most common use would be to change the
> aspect ration without the need for re-encoding. Some MOD files have
> the aspect ratio incorrectly set to 4:3
> (see https://en.wikipedia.org/wiki/MOD_and_TOD).

Thanks for the contribution, but this looks like a limited version of
https://lists.libav.org/pipermail/libav-devel/2017-July/084528.html ,
which seems to contain support for MPEG-2 bit stream filtering as
well.

Please see if you can cherry-pick this patch set and test with it. The
framework posted on libav-devel seems more complete and seems to
support other formats than just MPEG-2 video. If it works for you,
feel free to post the patch set here as well.

Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] Add tonemap filter

[Jan Ekstrom]

On Tue, Jul 18, 2017 at 10:33 PM, Vittorio Giovara
 wrote:
> Based off mpv automatic tonemapping capabilities.

Cool stuff, will have to test this soon. Thanks for putting an example
in the docs.

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideodec: decode to GBR(A)P

[Jan Ekstrom]

Hi,

On Mon, Jun 26, 2017 at 2:12 PM, Paul B Mahol  wrote:
>
> Yes it is.

As the output is verified to be bit-exact, this is LGTM for me (as
someone who has poked this format's dec/enc in the past).

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

Re: [FFmpeg-devel] [PATCH 2/2] avcodec: remove anatoliy prores encoder

[Jan Ekstrom]

On Mon, Jun 26, 2017 at 5:09 PM, Paul B Mahol  wrote:
> Rationale:
> - Slower then other encoder
> - Less configurable
> - Does not support alpha profile
> - Does not set interlaced flag
> - Worse output quality
> - No need for 2 encoders
>

I agree with this clean-up. Probably needs an API/ABI bump?

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

Re: [FFmpeg-devel] [PATCH 1/2] avfilter/proresenc: switch default prores encoder to prores_ks

[Jan Ekstrom]

On Mon, Jun 26, 2017 at 5:09 PM, Paul B Mahol  wrote:
> Rationale:
> prores_ks have more features and is faster for qscale > 0
> and gives better quality output.
>

You probably meant s/avfilter/avcodec/ :) All those filters you've
made have put a template in your mind ;) .

Generally LGTM for me. Does this need an API/ABI bump?

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

Re: [FFmpeg-devel] Point Cloud Compression

[Jan Ekstrom]

On Wed, Mar 8, 2017 at 10:23 PM, Toepfer, Randall  wrote:
> Has anyone used FFMPEG for point cloud mpeg encoding and streaming?  If not
> does FFMPEG support this?
>
> http://mpeg.chiariglione.org/standards/exploration/point-cloud-compression
>

From the document "Call for Proposals for Point Cloud Compression":
>  The first working draft and test model - 2017.10.27

So, uh, yeah.


Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libx264: remove the "x264opts" AVOption

[Jan Ekstrom]

On Wed, Mar 8, 2017 at 1:01 AM, Michael Niedermayer
 wrote:
> the fast pskip i mentioned yesterday
>
> ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264-params 
> fast-pskip -vframes 15 -an ref-p.nut
> ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264-params 
> no-fast-pskip -vframes 15 -an ref-np.nut
> ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264opts 
> no-fast-pskip -vframes 15 -an ref-no.nut
> ./ffmpeg -y -i ~/videos/matrixbench_mpeg2.mpg -vcodec libx264 -x264opts 
> fast-pskip -vframes 15 -an ref-o.nut
>
> ffd2c735cfb268d5f4291c2f6baab386  ref-o.nut
> ffd2c735cfb268d5f4291c2f6baab386  ref-p.nut
> f424075a964018aa83f98f2bf5ab2830  ref-no.nut
> ffd2c735cfb268d5f4291c2f6baab386  ref-np.nu
>
> it appears *fast-pskip has no effect in x264-params

Right, as I noted in my previous message, the custom dictionary
parsing code in there sets the value to "1" if a value is not
available. The API always expects a key and a value (even if a NULL
value would be valid for a boolean option).
So to emulate what x264opts is doing you would just set "fast-pskip=1"
or "no-fast-pskip=1". You can by this guess that fast-pskip is indeed
the default behavior during medium preset in libx264.

In my personal opinion I'm not sure this is worth it to have a Yet
Another Dictionary Parser inside libx264.c, esp. since
av_dict_parse_string() behavior is already the defining one in
multiple components where key/value lists are handled (you can see our
Doxygen for some sort of list of usage:
https://www.ffmpeg.org/doxygen/trunk/group__lavu__dict.html#gaca5ff7c251e60bd13164d13c82f21b79
).

>
> i have no idea if thats the only issue
>

Unless the very short code around it, or av_dict_parse_string is
globally bugged in some way, most likely all "issues" with x264-params
are due to the implicit value being set for key-value pairs without a
value.

> also to keep both x264-params and x264opts working you need just
> 1 line more than if you drop one. Its just one entry in the AVOption
> array to pass both to the same implementation
>

That is another alternative and at the very least would remove the
duplicated implementation which would be a positive step forward. For
some reason I would prefer only having a single option for this so
that there would be no questions regarding why the behavior changed
for x264opts with regards to implicit values.


Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] libx264: remove the "x264opts" AVOption

[Jan Ekstrom]

On Tue, Mar 7, 2017 at 5:24 AM, Michael Niedermayer
 wrote:
> On Tue, Mar 07, 2017 at 12:20:20AM +0200, Jan Ekström wrote:
>> x264-params does the same thing and this leaves us with a single
>> option that is name-matched with x265-params available in the
>> libx265 wrapper.
>> ---
>>  libavcodec/libx264.c | 29 -
>>  1 file changed, 29 deletions(-)
>
> please wait with this
>

The patch set has [RFC] in its topic as I knew any sort of removal is
going to be a multi-step process. As I noted, I wasn't sure of the
process so I noted that in my cover letter.

> ...
> its a long time ago but i faintly remember that the option you are
> about to remove worked while the one remaining had bugs

The x264opt code seems to:

* Use custom dict parsing instead of av_dict_parse_string
* Try real hard to set you a "1" even if you set something else:

>  char param[256]={0}, val[256]={0};
>  if(sscanf(p, "%255[^:=]=%255[^:]", param, val) == 1){
>  OPT_STR(param, "1");
>  }else
>  OPT_STR(param, val);

* The OPT_STR macro tries to catch X264_PARAM_BAD_NAME, while
x264-params just outputs "Error parsing option '%s = %s'.\n" in all
failure cases of x264_param_parse.

The x264-params code seems to:

* Use av_dict_parse_string/av_dict_get

>  av_dict_parse_string(, x4->x264_params, "=", ":", 0)
>  en = av_dict_get(dict, "", en, AV_DICT_IGNORE_SUFFIX)

* Just pass key=value one by one into x264_param_parse

>  x264_param_parse(>params, en->key, en->value)


Personally I prefer the latter as it utilizes generic tools we have in
our libraries and seems to keep the idea closer to KISS. Catching
X264_PARAM_BAD_NAME might be a good addition to x264-params, but not
sure how much more helpful that would be in the end.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] doc/encoders: remove mentions of the x264opts AVOption

[Jan Ekstrom]

On Tue, Mar 7, 2017 at 12:44 AM, Marton Balint  wrote:
> I guess you meant to convert this to -x264-params as well, no?
>
> Thanks,
> Marton

Yup, welcome to doing things after midnight.

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

Re: [FFmpeg-devel] HEVC Video Transcode Transfer VUI, SEI Information

[Jan Ekstrom]

On Fri, Mar 3, 2017 at 4:38 AM, Ben Chang  wrote:
>
> In short, is there any way to transfer meta data between a decode and encode 
> context in transcode scenario? If not, would it be supported in foreseeable 
> future?
>

Hi,

AVFrames do contain fields for:
* color_primaries
* color_trc
* colorspace
... and then there is the "new" side data type for the mastering
display data (AVMasteringDisplayMetadata).

So in theory if the decoder exports that information (AVC supports at
least the first three, and HEVC the latter side data as well) and you
utilize those values of the AVFrame in the encoder module, that should
be possible to obtain. Also I think AVFilter also takes in AVFrames,
which makes utilization of such information and updating it throughout
the chain possible when using the libav* framework.

Also, the recently merged better initialization of inputs in ffmpeg.c
might possibly help with some formats if ffmpeg.c is utilized and the
components involved support using the information within the AVFrames.

Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] movenc: add support for track names in ISML manifests

[Jan Ekstrom]

On Sat, Feb 11, 2017 at 1:21 AM, Jan Ekström  wrote:
> ...
>

Poked Martin about this last night, and his comment was:
`<@wbs> JEEB: doesn't look harmful to me, so no objection`

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

Re: [FFmpeg-devel] [PATCH] lavf/mov: Add support for edit list parsing.

[Jan Ekstrom]

On Fri, Oct 21, 2016 at 6:32 PM, Derek Buitenhuis
 wrote:
> The design one fist: This approach is fundamentally wrong. Edit lists are
> meant to be applied at presentation level, not packet level. The current
> implementation will cause problems in a number of cases:
>
> * Audio packets. Especially audio packets with a large number of samples.
>   It's extremely likely that edits will not fall on packet boundaries, and
>   depending on the number of samples per packet, audio sync issues can and
>   will occur, even with smaller packets of e.g. 1024 samples, if there are
>   a large number of entries in the edit list. Gradual loss of sync will
>   occur.
> * Edit list entries that are out of order, or repeat. This one is obvious;
>   simply dropping packets is not sufficient to create a virtual timeline
>   like edit lists can be used for by various NLEs. I need to look up
>   if these are actually allowed in the ISOBMFF of QT specs, but I know
>   Matroska allows it in its virtual timeline feature.
> * Returning timestamps that differ from the codec timestamps. I very
>   much disagree with this approach. It's the responsibility of the
>   presentation/render/transcode/app/whatever layer to adjust timestamps
>   based on edits. I absolutely disagree with libavformat changing
>   timestamps from what is coded in the actual file. avformat provides
>   demuxers.

Hi Sasi, Derek,

As I see people having issues with this implementation, is there an
existing structure we can utilize to export the edit list information?
(aka "can something like the chapters structure be utilized") If not,
what would be the requirements based on qtff and ISOBMFF for virtual
timelines? Does the demuxer have such a structure already in place
after which the new metadata for virtual timelines could be based on?

We will have to take baby steps towards getting this stuff right. The
first step IMHO would be to export the edit list from the demuxer so
that the presentation layer can utilize its information. Hints on
where this information should be gathered from now that (I think) we
have the parsing code in the demuxer in place are very welcome as
well.

After we get the edit list export right, we will have to try and
integrate it into the mess that is ffmpeg.c.

Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] TR-03 implementation

[Jan Ekstrom]

On Sun, Nov 13, 2016 at 9:48 AM, Carlos Fernandez Sanz
 wrote:
> One of the reasons apparently for not merging the SCTE-35 patch;
> better never include that in ffmpeg and have the professional use and
> pay for other products even though an implementation that doesn't
> break anything at that has been revised 14 times has been posted here.
>
> Reason not to merge? None, that just it "doesn't belong to FFmpeg".
> And trying to merge it is not mature.

Hi,

This is completely unrelated to this specific thread, so please take
it up there in that thread (and reply to the following parts of this
e-mail there, please). Although I remember that the issue with
timestamps not being usable yet was one of the largest blockers? The
whole thing of "should this be a 'codec' or should it be side data" is
IMHO also not insignificant, but if I recall correctly the timestamp
ordeal was one of the major reasons for this thing not yet being in. I
can recall incorrectly, of course. So you can reply in that thread
quoting me if you feel like something's incorrect that I've said.

As for this thread, we have already pretty much agreed both on this ML
and on IRC that Ros's reply was out of line. He has apologized. The
point that people were trying to make is that for the person who is
going to be implementing this, the actual pixel format part is the
least of one's concerns (not insignificant, but definitely something
that can be solved in "one place" with enough SIMD etc). The RTP input
module being sub-optimal is the more major issues (among others) -
together with the pure amount of data in general that would go through
the network - which might require a rework on a much higher level of
how the input framework works. And that generally tends to be a much
higher hurdle to get over. If someone gets it done properly in the
various areas, all the more power to them, but the scope of where the
person will most probably have most of their issues should be noted if
already known by someone who has done related work. Additionally,
depending on the needs of a use case other alternatives (which tend to
use parts of FFmpeg for specific things like decoding encoded video or
encoding it) such as upipe can be a simpler place to start (due to
base design assumptions being different than with libavformat f.ex.).

What I most definitely am not seeing is people trying to push others
to proprietary solutions.

Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] add hds demuxer

[Jan Ekstrom]

On Mon, Oct 31, 2016 at 5:30 PM, Nicolas George  wrote:
> Le nonidi 9 brumaire, an CCXXV, Steven Liu a écrit :
>>  I saw ffmpeg have no HDS and DASH demuxer, and all of them's format is
>> use xml, maybe this parser is a very useful parser, what about the basic
>> xml :-D
>
> The Timed Text Markup Language, a subtitle format used by Youtube and
> possibly a few others, is based on XML too.
>
> I have started working on a simple XML parser, but Michael quickly found
> a bug in my attempt to remove the recursiveness in libavfilter, and I
> consider it to be the highest priority. Therefore, I stopped shortly
> after implementing the framework API and input buffer handling.
>

Hi,

As someone who thought about doing some work on formats that require
an XML parser, while I do appreciate that you are making a "simple"
XML parser, I am not sure if this is the best way forward. XML and
thus by continuation libraries that handle XML are indeed
abominations, but I am definitely not sure if we should be NIH'ing XML
parsing. For example, namespaces are already utilized in DASH/CENC.
Maybe we should just pick one XML parsing library that seems to be the
least bad of all bad alternatives, and then standardize FFmpeg on it?
Be it libxml2, libexpat or anything else?

Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] lavf/mxfdec: begin utilizing the newly parsed widths and heights

[Jan Ekstrom]

On Sep 26, 2016 04:05, "Marton Balint"  wrote:
>
> Overriding width/height with display width/height does not seem right, check 
> what happens with a PAL IMX50 MXF file for example. If you want to signal 
> this, then a stream side data with some AVPanScan values might make more 
> sense.
>

Such a file was actually the reason why I started looking into this :)
. And it would all depend on the definition of width/height in
codecpar, which as far as I can tell is not clearly cut (see the notes
regarding container crop in at least the AVC decoder, I think?). My
understanding was that it would be the displayed width/height. Of
course, the container cropping/padding makes this less simple, since
you have:

1) Decoded picture
2) Decoder cropped picture (what the decoders *currently* output)
3) Decoder cropped picture cropped/padded according to available metadata

So my understanding was - since a decoder should output the pictures
according to 3) - that the displayed width/height fields should be
utilized for signaling the final display width/height of the picture.
The X/Y offset handling of course should have had its own fields
*somewhere* so that stuff could be done in some common part of avcodec
(for example). But if the codecpar->width/height is specified to 2)
(at least in lavf), then of course side data would be the correct way
to handle this. Also thanks for the hint regarding AVPanScan, I had no
idea this existed. Will have to check if it actually is used anywhere.

Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 1/2] lavf/mxfdec: add support for all of the picture size and offset fields

[Jan Ekstrom]

On Sep 26, 2016 09:29, "Carl Eugen Hoyos"  wrote:
>
> 2016-09-26 1:52 GMT+02:00 Jan Ekström :
>
> If this fixes anything (I am not sure I understand: Are you changing a
> type which introduces a possible undefined behaviour, so you add an
> additional check at the same time?) it should be part of above new
> patch imo (or an independent patch).
>

The specification notes that width and height are uint32 in MXF. Thus the
old type of "int" was incorrect. The width and height of codecpar are "int"
so by fixing the type it can overflow (you'd get negative values), and thus
have to be capped.

The old code would have had thus an overflow happen naturally in the case
of a large enough unsigned integer as the value would have been read into
an int.

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

Re: [FFmpeg-devel] [PATCH] doc/developer.texi: Add a code of conduct

[Jan Ekstrom]

On Wed, May 18, 2016 at 9:40 PM, Michael Niedermayer
 wrote:
> Signed-off-by: Michael Niedermayer 
> ---
>  doc/developer.texi |   29 +
>  1 file changed, 29 insertions(+)
>

I agree to this addition. I think having this is a good initial
stepping stone for any future improvements.

Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] pgssubdec: fix subpicture output colorspace and range

[Jan Ekstrom]

On Sat, Apr 23, 2016 at 3:21 PM, wm4  wrote:
> In that case the hdtv field should be completely removed, and the test
> put in the palette conversion code.
>

If avctx->height is by default 0, then it would have to be something a la:

if (!avctx->height || avctx->height > 576) {
YUV_TO_RGB1_CCIR_BT709(cb, cr);
} else {
YUV_TO_RGB1_CCIR(cb, cr);
}

I guess? If the default is -1 then it'd be a check for <0 or >576.
This way even if in a cut file a palette comes up before a
presentation segment it should default to BT.709.

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

Re: [FFmpeg-devel] [PATCH] pgssubdec: fix subpicture output colorspace and range

[Jan Ekstrom]

On Sat, Apr 23, 2016 at 11:09 AM, Carl Eugen Hoyos  wrote:
> Please mention the relevant ticket in the commit message.

OK, will do.

>
>> +int hdtv;
>
> Please rename to sdtv so you can remove the assignment from
> init_decoder().
>

What would that make of the default value? The current design lets me
test for C true/false since the default is overridden at init() to -1
and the HD content would have 1 set. I seem to remember that in stack
various values are set to zero, but is the context always on stack?

>
> ctx->sdtv = h <=576 (or add braces).
>

OK, will set the value according to the response to my previous question.

>> +#define YUV_TO_RGB1_CCIR_BT709(cb1, cr1)\
>
> Just being curious: Did you test how much the values were
> off without this hunk?
>

The usual amount of BT.601 vs BT.709. Granted, many subpictures don't
contain much if any color, but since the values are not really limited
I'd say it's only fair to compare it to a standard BT.709/BT.601
rendering difference. If you haven't experienced such difference yet,
you can test it with these following files that contain the exact same
YCbCr values, but different colorimetry metadata in the bit stream
(601 and 709 accordingly):
http://cccp-project.net/beta/test_files/renderer_test_with_bt601.mkv
http://cccp-project.net/beta/test_files/renderer_test_with_bt709.mkv

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

Re: [FFmpeg-devel] [PATCH] pgssubdec: fix subpicture output colorspace and range

[Jan Ekstrom]

On Sat, Apr 23, 2016 at 2:53 PM, Hendrik Leppkes  wrote:
> Otherwise, I think Carl's suggestion might help, PGS subtitles are
> generally from Blu-rays, which means most of it is HD, so swapping the
> flag to detect SD conditions might make it act more "appropriate" in
> absence of w/h.

The default is -1 and I have switched the check for BT.709 to `if
(ctx->hdtv)` so it should only use BT.601 if and only if it gets a
presentation segment with a height that is <= 576.

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

Re: [FFmpeg-devel] [PATCH]lavc/pgssubdec: Fix palette colourspace

[Jan Ekstrom]

On Tue, Apr 19, 2016 at 3:15 PM, Carl Eugen Hoyos  wrote:
> Could you explain how I can reproduce the incompleteness?
> Visibly if possible...

It seems right now that we have a function (macro) to convert
(seemingly limited range?) YCbCr to limited range or full range RGB,
both (seemingly) following Rec.601. This is correct for the <= 576
height case. We thus need another one that does the same thing, but
with Rec.709 coefficients for the >=720 height case. Reimar did note
an example in his message, although it would be a good idea to get the
values checked against Rec.709 (I wish there was one place to have
such value/plane conversion functions so we could just plug into it
and not have to add more code, but alas we seem to have these things
all around the place).

HDMV subpictures do let you have a wide variety of YCbCr values in the
palette so the conversion for the overlay does matter if we want to
output RGB.

Cheers,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavc/pgssubdec: Fix palette colourspace

[Jan Ekstrom]

On Sun, Apr 17, 2016 at 10:08 PM, Carl Eugen Hoyos  wrote:
>
> Does attached make it better?

So the difference between the two conversion functions is that one is
Rec.601 in limited range, and the other is Rec.709 in limited range?
If so, that seems correct. The actual coded width/height of "625/50"
can only be 720x576, so you can modify the check for that height :) .

That said, what Reimar noted about blending could be true, but that's
an even larger issue in general. I couldn't quickly find the place
where they define in which color space the alpha blending should be
done in, but given the close proximity of the colorimetry I wouldn't
be surprised if it was meant to be done in YCbCr.


Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavc/pgssubdec: Fix palette colourspace

[Jan Ekstrom]

On Sun, Apr 17, 2016 at 9:55 PM, Reimar Döffinger
 wrote:
> ?!???
> These two kind of contradict each other, at least if the HDMV video
> stream uses a full range color matrix (or is that not allowed?).

Just poked at this and it seems indeed that the following values are
set for the resolution profiles for AVC:
- 525/60: primaries|transfer|matrix => 6 (and implied if description
is not available), full range shall be set to zero
- 625/50: primaries|transfer|matrix => 5 (=||=), =||=
- 1080: primaries|transfer|matrix => 1 (=||=), =||=
- 720: primaries|transfer|matrix => 1 (=||=), =||=

So yes, it seems relatively safe to expect the conversion to be
according to these values according to the fact that the "canvas" has
a defined colorimetry per resolution.

Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavc/pgssubdec: Fix palette colourspace

[Jan Ekstrom]

On Sun, Apr 17, 2016 at 9:44 PM, wm4  wrote:
>
> Ah that's funny. This indeed ruins everything. It looks like subtitle
> decoders should simply output YUV, and until we can fix it, a private
> option could change between the colorspaces?

Well, the colorimetry is at least specified as per the resolution, and
while I haven't confirmed it I would guess you could decide things by
the size of the "canvas", since it seems like SD content has been
deemed Rec.601, while HD content has been deemed Rec.709).

Of course, I do agree that enabling YCbCr output could be one way of
getting through this for some users of the API.

Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavc/pgssubdec: Fix palette colourspace

[Jan Ekstrom]

On Sun, Apr 17, 2016 at 9:21 PM, Reimar Döffinger
 wrote:
> In particular, I have an uncomfortable suspicion that
> PGS might be designed to match the movie's colour space,
> in which case neither variant would give correct results
> but instead it would have to depend on what format the
> corresponding video track uses (and it probably would
> be more than just the 255/224 factor that would differ).

Yes, the YCbCr values in palettes are matched accordingly against the
video stream. As per the specification:
"Y, Cr and Cb shall have the same color matrix as the associated HDMV
Video stream: 525-60/625-50 (Rec.601); 1080i, 720p (ITU-709)"

Otherwise the specification notes that valid values in palettes are:
-Y: 16-235
-Cr: 16-240
-Cb: 16-240
-Transparency: 0-255, where a value of ‘255’ indicates full opacity
and ‘0’ indicates full transparency.
(Each Palette entry shall have a default transparency value of 0; the
default values of Y, Cr and Cb are undefined.)


Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH]lavc/pgssubdec: Fix palette colourspace

[Jan Ekstrom]

On Sun, Apr 17, 2016 at 6:25 PM, Carl Eugen Hoyos  wrote:
> Hi!
>
> Attached patch fixes ticket #4637 for me.
>
> Please comment, Carl Eugen

Hi,

Can you please explain the difference between these two YCbCr-to-RGB
conversion functions?

The result, if it is now black, seems to match what MPC-HC's subtitle
renderer does, which was recently (a year or two ago) updated to work
more according to the HDMV graphic stream specification.

I think wm4's primary concern is that the change is not backed by any
explanation regarding why it is incorrect or if this is even the right
place to fix the issue.

Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec: Remove libfaac, the internal AAC encoder is better

[Jan Ekstrom]

Hi,

On Sun, Apr 10, 2016 at 10:13 PM, Michael Niedermayer
 wrote:
> This is not about changing a bad encoder to a good encoder, this patch
> is about removing choices.
> Before this patch users can force libfaac and have twice as long
> battery life at lower quality after the patch the users do not have
> that choice anymore
>
> I do not think thats a good idea nor in the interrest of our users

I have thought about this somewhat, and the things boil down to:
* Libfaac is old, unmaintained, produces relatively bad quality and
requires a "nonfree" configuration, which disables any sort of binary
distribution. Last point probably being the most problematic for
anyone who wants to use it outside of a server context. In which case
there's already fdk-aac available, which has found immense popularity
during the last few years before the internal encoder became better.
Fdk-aac still serves a purpose with HE-AAC, as well as some specific
LC-AAC use cases (latter according to some random people on #ffmpeg ),
so it yet isn't considered something worth removing.
* Both are very fast (about 30x realtime vs 60x realtime as far as
could be gathered by the numbers posted on this thread if I am reading
them correctly). Even if you are doing live recording, neither of
these is likely to be slow enough for the CPU usage to really matter.
* The faac encoder will still be there for those who really want to
still use it, albeit no longer through libavcodec. This can actually
ease usage for some people as they can now compile libavcodec without
enable-nonfree and instead handle the licensing incompatibility on
their side in one way or another (except it's supposedly licensed as
GPL while parts of its source code are suposedly GPL-incompatible,
thus pretty much making that case not really true, unlike fdk-aac
which doesn't seem to have such contradictions within its own code
base).
* Keeping this encoder available will serve as an endorsement for it.
Do we really want to endorse this encoder?

Additionally, as a minor not-really-related anecdote, I have only seen
people on #ffmpeg having -c:a/-acodec libfaac on their command lines
when they have followed very old tutorials from up to just before
fdk-aac got published. They assume it's what they want to use due to
quality (which at this point is no longer true), and it used to be
distributed in various distributions linked to libavcodec - which is
no longer true in many distributions, such as Ubuntu.

In any case, I think my opinion on this boils to this component not
really being in any way good enough that we should keep this
alternative around, including but not limited to its license
situation. Old versions will keep having it, and if someone knowingly
wants to use it specifically through libavcodec, they can still do so.
Otherwise, they can stick it into their media workflow in other ways,
as I noted. I do try to understand the wish to keep choices available
to users, but in this case I am not sure if it brings in anything else
but confusion.

Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 3/3] movenc: simplify the 'tfxd' fragment start timestamps

[Jan Ekstrom]

On Sun, Mar 20, 2016 at 3:28 PM, Michael Niedermayer
 wrote:
> On Sat, Mar 19, 2016 at 07:39:07PM +0200, Jan Ekström wrote:
>> As far as can be seen, this value is supposed to be the DTS of a
>> fragment in smooth streaming. Thus, don't take b-picture delay and
>> such into mention when calculating the start timestamp. The duration
>> calculation requires PTS values, so it is not touched.
>> ---
>>  libavformat/movenc.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
>> index 11c8275..77f28b0 100644
>> --- a/libavformat/movenc.c
>> +++ b/libavformat/movenc.c
>> @@ -3565,8 +3565,7 @@ static int mov_write_tfxd_tag(AVIOContext *pb, 
>> MOVTrack *track)
>>  avio_write(pb, uuid, sizeof(uuid));
>>  avio_w8(pb, 1);
>>  avio_wb24(pb, 0);
>> -avio_wb64(pb, track->start_dts + track->frag_start +
>> -  track->cluster[0].cts);
>> +avio_wb64(pb, track->start_dts + track->frag_start);
>>  avio_wb64(pb, track->end_pts -
>>(track->cluster[0].dts + track->cluster[0].cts));
>
> breaks fate / needs an update for the checksums if the changes match
> what is intended
>

As far as I can tell this vendor-specific UUID box is a variant of the
"fragment DTS" box specified in ISOBMFF.

Streaming servers I've dealt with seem to parse the initial
timestamp of a fragment as follows:
[timestamp in tfxd] + [CTS offset of the initial sample of the fragment]
Now, if you keep this part of it as-is, you will have the
tfxd timestamp be larger by the CTS offset *and* you will
have a CTS offset in the initial fragment, effectively
doubling the delay.

This fixes one part of the delay, the second part would be
to add support for negative CTS offsets in the "trun"
boxes, which would enable the DTS and CTS be the same for
the initial sample of a fragment (CTS offset of zero). I have a
POC of this lying on my repository, but last I tried it didn't
exactly work perfectly, so I will have to look into that some more.

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

Re: [FFmpeg-devel] [PATCH] rtmpdh: Initialize gcrypt before using it

[Jan Ekstrom]

Hi,

In general looks good, although it might look a bit weird for someone
as usually libraries have initialization functions called like that.
That said, this is what
https://gnupg.org/documentation/manuals/gcrypt/Initializing-the-library.html
recommends.

My only comment would be that we might want to set the parameter to
GCRYPT_VERSION instead of NULL, as we most probably care if the
library loaded matches our version (unless these versions change even
if API doesn't change).

Best regards,
Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] support for reading / writing encrypted MP4 files

[Jan Ekstrom]

On Mon, Dec 14, 2015 at 10:48 PM, compn  wrote:
> dumb question
>
> are these encrypted mp4 files some kind of standard encryption?
>
> rephrased... will encrypted files created by ffmpeg be able to be
> decrypted/decoded and played by quicktime? or any other player?
> (assuming other player has the keys of course).

If the patches follow the specification noted @
https://w3c.github.io/encrypted-media/cenc-format.html (standardized @
ISO as 23001-7 |
http://www.iso.org/iso/home/store/catalogue_ics/catalogue_detail_ics.htm?csnumber=65271
as far as I can tell), then yes - the encryption itself is standard.
Usually this sort of encryption is coupled with MPEG-DASH where you
stick the related information on how to get the keys into
ContentProtection blocks in the manifests.

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

Re: [FFmpeg-devel] UDP constant bitrate feature (cbr)

[Jan Ekstrom]

Hi,

On Wed, Nov 18, 2015 at 9:28 PM, Zach Swena  wrote:
> Are you referring to this seperate applciation?
>
> https://github.com/mmalecki/multicat/blob/master/trunk/README
>

Probably what was meant was
http://www.videolan.org/projects/multicat.html , but that might be a
git svn clone of it.

>
> ...I would argue that this very
> much is a bug, not a feature.  Just because an external applciation can fix
> the problem in some instances doesn't mean it isn't a problem in the first
> place.
>

I do not think anyone in this thread was noting that using another
application is the preferred solution and that it shouldn't be
improved in FFmpeg. Just that multicat is doing what is considered to
be correct (and if any fixes are going to be done they should be done
in that manner).


Best regards,
Jan Ekström
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel