Re: [libav-devel] [PATCH] dca: Move the downmix request check outside the loop

[Vittorio Giovara]

On Mon, Apr 24, 2017 at 4:41 PM, Luca Barbato  wrote:
> From: Anton Khirnov 
>
> ---
>
> An easy part of the whole ch_layout set we are shaping up.
>
>  libavcodec/dcadec.c | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/libavcodec/dcadec.c b/libavcodec/dcadec.c
> index 3fe46cd..9c1f878 100644
> --- a/libavcodec/dcadec.c
> +++ b/libavcodec/dcadec.c
> @@ -932,7 +932,7 @@ static int dca_subsubframe(DCAContext *s, int 
> base_channel, int block_index)
>  return 0;
>  }
>
> -static int dca_filter_channels(DCAContext *s, int block_index, int upsample)
> +static int dca_filter_channels(DCAContext *s, int block_index, int upsample, 
> int downmix)
>  {
>  int k;
>
> @@ -1000,8 +1000,7 @@ static int dca_filter_channels(DCAContext *s, int 
> block_index, int upsample)
>  /* FIXME: This downmixing is probably broken with upsample.
>   * Probably totally broken also with XLL in general. */
>  /* Downmixing to Stereo */
> -if (s->audio_header.prim_channels + !!s->lfe > 2 &&
> -s->avctx->request_channel_layout == AV_CH_LAYOUT_STEREO) {
> +if (downmix) {
>  dca_downmix(s->samples_chanptr, s->amode, !!s->lfe, s->downmix_coef,
>  s->channel_order_tab);
>  }
> @@ -1378,6 +1377,7 @@ static int dca_decode_frame(AVCodecContext *avctx, void 
> *data,
>  DCAContext *s = avctx->priv_data;
>  int channels, full_channels;
>  int upsample = 0;
> +int downmix;
>
>  s->exss_ext_mask = 0;
>  s->xch_present   = 0;
> @@ -1488,6 +1488,9 @@ static int dca_decode_frame(AVCodecContext *avctx, void 
> *data,
>  return ret;
>  }
>
> +downmix = s->audio_header.prim_channels > 2 &&
> +  avctx->request_channel_layout == AV_CH_LAYOUT_STEREO;
> +
>  /* filter to get final output */
>  for (i = 0; i < (s->sample_blocks / SAMPLES_PER_SUBBAND); i++) {
>  int ch;
> @@ -1497,7 +1500,7 @@ static int dca_decode_frame(AVCodecContext *avctx, void 
> *data,
>  for (; ch < full_channels; ch++)
>  s->samples_chanptr[ch] = s->extra_channels[ch - channels] + i * 
> block;
>
> -dca_filter_channels(s, i, upsample);
> +dca_filter_channels(s, i, upsample, downmix);
>
>  /* If this was marked as a DTS-ES stream we need to subtract back- */
>  /* channel from SL & SR to remove matrixed back-channel signal */
> --

ok
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [PATCH] dca: Move the downmix request check outside the loop

[Luca Barbato]

From: Anton Khirnov 

---

An easy part of the whole ch_layout set we are shaping up.

 libavcodec/dcadec.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/libavcodec/dcadec.c b/libavcodec/dcadec.c
index 3fe46cd..9c1f878 100644
--- a/libavcodec/dcadec.c
+++ b/libavcodec/dcadec.c
@@ -932,7 +932,7 @@ static int dca_subsubframe(DCAContext *s, int base_channel, 
int block_index)
 return 0;
 }

-static int dca_filter_channels(DCAContext *s, int block_index, int upsample)
+static int dca_filter_channels(DCAContext *s, int block_index, int upsample, 
int downmix)
 {
 int k;

@@ -1000,8 +1000,7 @@ static int dca_filter_channels(DCAContext *s, int 
block_index, int upsample)
 /* FIXME: This downmixing is probably broken with upsample.
  * Probably totally broken also with XLL in general. */
 /* Downmixing to Stereo */
-if (s->audio_header.prim_channels + !!s->lfe > 2 &&
-s->avctx->request_channel_layout == AV_CH_LAYOUT_STEREO) {
+if (downmix) {
 dca_downmix(s->samples_chanptr, s->amode, !!s->lfe, s->downmix_coef,
 s->channel_order_tab);
 }
@@ -1378,6 +1377,7 @@ static int dca_decode_frame(AVCodecContext *avctx, void 
*data,
 DCAContext *s = avctx->priv_data;
 int channels, full_channels;
 int upsample = 0;
+int downmix;

 s->exss_ext_mask = 0;
 s->xch_present   = 0;
@@ -1488,6 +1488,9 @@ static int dca_decode_frame(AVCodecContext *avctx, void 
*data,
 return ret;
 }

+downmix = s->audio_header.prim_channels > 2 &&
+  avctx->request_channel_layout == AV_CH_LAYOUT_STEREO;
+
 /* filter to get final output */
 for (i = 0; i < (s->sample_blocks / SAMPLES_PER_SUBBAND); i++) {
 int ch;
@@ -1497,7 +1500,7 @@ static int dca_decode_frame(AVCodecContext *avctx, void 
*data,
 for (; ch < full_channels; ch++)
 s->samples_chanptr[ch] = s->extra_channels[ch - channels] + i * 
block;

-dca_filter_channels(s, i, upsample);
+dca_filter_channels(s, i, upsample, downmix);

 /* If this was marked as a DTS-ES stream we need to subtract back- */
 /* channel from SL & SR to remove matrixed back-channel signal */
--
2.9.2

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

Re: [libav-devel] [RFC] Getting rid of non-native endian pixfmts

[Luca Barbato]

On 4/24/17 7:19 PM, wm4 wrote:
> On Mon, 24 Apr 2017 18:34:41 +0200
> Luca Barbato  wrote:
> 
>> On 4/24/17 5:37 PM, wm4 wrote:
>>> I propose that we remove non-native endian pixfmt variations. Pixel
>>> data would always be in native endianness (for example little endian on
>>> a little endian system). All LE/BE formats would be dropped, and only
>>> those without suffix would be left (mostly this implies moving the
>>> current native-endian aliases to the proper pixfmt enum).
>>>
>>> Some formats (mostly simply raw formats) actually store data in a
>>> specific endianness - decoders and encoders would need to byte-swap
>>> them.  
>>
>> We have a tiny amount of codecs with the issue (e.g. tiff), we would
>> have some command line incompatibility and probably we could just add a
>> be/le option for the specific codecs instead of having the RAW encoder
>> follow PCM in having all the possible combinations explicitly set.
> 
> Not sure what the command line matters. If users explicitly force the
> pixfmts for encoders which can't select the target pixfmt implicitly?

The only use-case I'm considering plausible is generating a series of
tiff images on a specific format that is required by some other program.

> Maybe be/le formats could be made aliases at some point, although that
> doesn't really seem like a good solution.

Usability wise if we go to he 1-codec-per-raw-format it is the same as
we do for pcm so it is sort of least surprising, if we go the option
route we'd have an option for the raw encoder and tiff (and those I'm
forgetting). A little more surprising, but we wouldn't clutter the
codecs list with seldom used raw formats.

>> An open question could be what's the proper way to send the information
>> from the demuxer (e.g. mkv with the new raw support) to our swapping raw
>> decoder.
> 
> Like with sample formats: as a codec, or codec parameter.

I'd rather use a codec parameter.

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] Getting rid of non-native endian pixfmts

[Vittorio Giovara]

On Mon, Apr 24, 2017 at 1:32 PM, wm4  wrote:
> On Mon, 24 Apr 2017 20:27:28 +0300
> Rémi Denis-Courmont  wrote:
>
>> Le maanantaina 24. huhtikuuta 2017, 17.37.00 EEST wm4 a écrit :
>> > I propose that we remove non-native endian pixfmt variations. Pixel
>> > data would always be in native endianness (for example little endian on
>> > a little endian system). All LE/BE formats would be dropped, and only
>> > those without suffix would be left (mostly this implies moving the
>> > current native-endian aliases to the proper pixfmt enum).
>>
>> I violently agree.
>>
>> > Some formats (mostly simply raw formats) actually store data in a
>> > specific endianness - decoders and encoders would need to byte-swap
>> > them.
>>
>> Yes.
>>
>> I also advocate eliminating palette-based formats likewise.
>
> We probably can't get rid of AV_PIX_FMT_PAL8 (or can we?), but we can
> definitely drop AV_PIX_FMT_FLAG_PSEUDOPAL (it's complete nonsense).

on behalf of two coworkers of mine: YES PLEASE
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] Getting rid of non-native endian pixfmts

[Vittorio Giovara]

On Mon, Apr 24, 2017 at 11:37 AM, wm4  wrote:
> I propose that we remove non-native endian pixfmt variations. Pixel
> data would always be in native endianness (for example little endian on
> a little endian system). All LE/BE formats would be dropped, and only
> those without suffix would be left (mostly this implies moving the
> current native-endian aliases to the proper pixfmt enum).
>
> Some formats (mostly simply raw formats) actually store data in a
> specific endianness - decoders and encoders would need to byte-swap
> them.
>
> Reasons pro:
> - this would dramatically lower the number of pixfmts
> - audio formats are always native endian too

these are the two most important selling points for me, so i would favor this
it would still be nice to have the possibility to output different
endianness formats but that can be done elsewhere
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] Getting rid of non-native endian pixfmts

[Jean-Baptiste Kempf]

Hello,

On Mon, 24 Apr 2017, at 08:37, wm4 wrote:
> I propose that we remove non-native endian pixfmt variations. Pixel
> data would always be in native endianness (for example little endian on
> a little endian system). All LE/BE formats would be dropped, and only
> those without suffix would be left (mostly this implies moving the
> current native-endian aliases to the proper pixfmt enum).

Please do this.

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] Getting rid of non-native endian pixfmts

[wm4]

On Mon, 24 Apr 2017 20:27:28 +0300
Rémi Denis-Courmont  wrote:

> Le maanantaina 24. huhtikuuta 2017, 17.37.00 EEST wm4 a écrit :
> > I propose that we remove non-native endian pixfmt variations. Pixel
> > data would always be in native endianness (for example little endian on
> > a little endian system). All LE/BE formats would be dropped, and only
> > those without suffix would be left (mostly this implies moving the
> > current native-endian aliases to the proper pixfmt enum).  
> 
> I violently agree.
> 
> > Some formats (mostly simply raw formats) actually store data in a
> > specific endianness - decoders and encoders would need to byte-swap
> > them.  
> 
> Yes.
> 
> I also advocate eliminating palette-based formats likewise.

We probably can't get rid of AV_PIX_FMT_PAL8 (or can we?), but we can
definitely drop AV_PIX_FMT_FLAG_PSEUDOPAL (it's complete nonsense).
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] Getting rid of non-native endian pixfmts

[wm4]

On Mon, 24 Apr 2017 18:34:41 +0200
Luca Barbato  wrote:

> On 4/24/17 5:37 PM, wm4 wrote:
> > I propose that we remove non-native endian pixfmt variations. Pixel
> > data would always be in native endianness (for example little endian on
> > a little endian system). All LE/BE formats would be dropped, and only
> > those without suffix would be left (mostly this implies moving the
> > current native-endian aliases to the proper pixfmt enum).
> > 
> > Some formats (mostly simply raw formats) actually store data in a
> > specific endianness - decoders and encoders would need to byte-swap
> > them.  
> 
> We have a tiny amount of codecs with the issue (e.g. tiff), we would
> have some command line incompatibility and probably we could just add a
> be/le option for the specific codecs instead of having the RAW encoder
> follow PCM in having all the possible combinations explicitly set.

Not sure what the command line matters. If users explicitly force the
pixfmts for encoders which can't select the target pixfmt implicitly?
Maybe be/le formats could be made aliases at some point, although that
doesn't really seem like a good solution.

> An open question could be what's the proper way to send the information
> from the demuxer (e.g. mkv with the new raw support) to our swapping raw
> decoder.

Like with sample formats: as a codec, or codec parameter.

> I need to think a bit more if breaking compatibility would impact the
> user in painful ways or it could be bearable.
> 
> Looking at -sample_fmts and -pix_fmts output I must say I find it quite
> interesting :)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] Getting rid of non-native endian pixfmts

[Luca Barbato]

On 4/24/17 5:37 PM, wm4 wrote:
> I propose that we remove non-native endian pixfmt variations. Pixel
> data would always be in native endianness (for example little endian on
> a little endian system). All LE/BE formats would be dropped, and only
> those without suffix would be left (mostly this implies moving the
> current native-endian aliases to the proper pixfmt enum).
> 
> Some formats (mostly simply raw formats) actually store data in a
> specific endianness - decoders and encoders would need to byte-swap
> them.

We have a tiny amount of codecs with the issue (e.g. tiff), we would
have some command line incompatibility and probably we could just add a
be/le option for the specific codecs instead of having the RAW encoder
follow PCM in having all the possible combinations explicitly set.

An open question could be what's the proper way to send the information
from the demuxer (e.g. mkv with the new raw support) to our swapping raw
decoder.

I need to think a bit more if breaking compatibility would impact the
user in painful ways or it could be bearable.

Looking at -sample_fmts and -pix_fmts output I must say I find it quite
interesting :)

lu
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] [PATCH] stereo3d: Update the naming API to be more consistent

[Vittorio Giovara]

On Mon, Apr 24, 2017 at 11:24 AM, James Almer  wrote:
> On 4/24/2017 11:55 AM, Vittorio Giovara wrote:
>> On Sun, Apr 23, 2017 at 11:31 PM, James Almer  wrote:
>>> On 4/20/2017 12:34 PM, Vittorio Giovara wrote:

 Change input type of the type->str function and return a proper
 error code for the str->type function.
 ---
 Similar reasoning to the previous patch.
 Vittorio

   libavutil/stereo3d.c | 6 +++---
   libavutil/stereo3d.h | 2 +-
   2 files changed, 4 insertions(+), 4 deletions(-)

 diff --git a/libavutil/stereo3d.c b/libavutil/stereo3d.c
 index 5dc902e909..6e2f9f3ad2 100644
 --- a/libavutil/stereo3d.c
 +++ b/libavutil/stereo3d.c
 @@ -54,9 +54,9 @@ static const char * const stereo3d_type_names[] = {
   [AV_STEREO3D_COLUMNS] = "interleaved columns",
   };
   -const char *av_stereo3d_type_name(unsigned int type)
 +const char *av_stereo3d_type_name(enum AVStereo3DType type)
   {
 -if (type >= FF_ARRAY_ELEMS(stereo3d_type_names))
 +if ((unsigned) type >= FF_ARRAY_ELEMS(stereo3d_type_names))
   return "unknown";
 return stereo3d_type_names[type];
 @@ -72,5 +72,5 @@ int av_stereo3d_from_name(const char *name)
   return i;
   }
   -return -1;
 +return AVERROR(EINVAL);
>>>
>>>
>>> Why EINVAL here but ENOSYS for spherical?
>>
>> no reason, i can switch to enonsys
>
> I was thinking EINVAL is more correct for both cases.
>
>>
>>> Also, while changing -1 to AVERROR() isn't a considerable API break as
>>> changing NULL on failure to a string on failure, it's still one and really
>>> makes me wonder if it's worth doing.
>>> While most users probably just check for < 0, in which case both return
>>> values would work the same, anyone instead checking explicitly for -1 (a
>>> completely valid check as per the documentation, which you for that matter
>>> did not update in this patch) will instead break.
>>
>> besides the fact that we're in the "break period" after the version
>> bump, there should be no reason to explicitly check for -1 so I don't
>> think that's a case that it's worth considering
>>
>
> As Anton already stated, there is no such thing as an API breaking
> season, only ABI breaking season. You can't change a function signature
> or return value/type on a whim if it can break downstream code.
> The documentation states the function returns -1 on failure. Explicitly
> checking for said value is a correct and completely valid way to check
> for failure. This patch would break such downstream code that follows
> the documentation to the letter.

That's not following documentation to the letter, is writing code that
it is not future proof.
In 99% of places using "ret < 0" is the valid and recommended way of
checking for errors, unless you need to validate against explicit and
documented values. A -1 is a terrible return value, and I'd rather
break the 1 or 2 theoretical uses (during the period where people need
to review their code anyway if they wish to continue using a recent
library) than propagating this wrong practice more.
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

[libav-devel] [RFC] Getting rid of non-native endian pixfmts

[wm4]

I propose that we remove non-native endian pixfmt variations. Pixel
data would always be in native endianness (for example little endian on
a little endian system). All LE/BE formats would be dropped, and only
those without suffix would be left (mostly this implies moving the
current native-endian aliases to the proper pixfmt enum).

Some formats (mostly simply raw formats) actually store data in a
specific endianness - decoders and encoders would need to byte-swap
them.

Reasons pro:
- this would dramatically lower the number of pixfmts
- actually facilitates endian-agnostic code, instead of bothering every
  downstream application to deal with endianness explicitly
- most real decoders always output native endianness (for example, the
  h264 decoder will never output AV_PIX_FMT_YUV420P10BE on a little
  endian system)
- most processing requires the data in native endianness anyway, so
  there will be a conversion
- raw formats could byte-swap the data while it's still in the cache, so
  it wouldn't affect speed negatively (or even speed it up)
- pixfmt endianness isn't even well defined - for example,
  AV_PIX_FMT_RGBA64BE requires endian-swapping _after_ extracting
  components, while AV_PIX_FMT_BGR565BE requires endian-swapping
  _before_ extracting them (or some horrible mixture). Pixdesc doesn't
  describe the difference very well either, and an API user has to
  guess either endian-dependent access is needed, since only the big
  endian variants have a flag set (!)
- as such it would simplify pixdesc
- can remove non-native endian paths from libswscale
- audio formats are always native endian too

Reasons contra:
- have to change all the raw decoders
- codec copy with raw-only formats could become slower

Thoughts?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] configure: Escape elements being filtered

[Diego Biurrun]

On Thu, Apr 06, 2017 at 06:14:29PM +0200, Luca Barbato wrote:
> The function is used to filter out ldflags arguments that may contain $.
> Update filter() as well to be consistent.

  configure: Properly escape arguments in filter/filter_out helper functions

  The arguments may contain '$', which gets interpreted by the shell.

OK

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] [PATCH] stereo3d: Update the naming API to be more consistent

[James Almer]

On 4/24/2017 11:55 AM, Vittorio Giovara wrote:
> On Sun, Apr 23, 2017 at 11:31 PM, James Almer  wrote:
>> On 4/20/2017 12:34 PM, Vittorio Giovara wrote:
>>>
>>> Change input type of the type->str function and return a proper
>>> error code for the str->type function.
>>> ---
>>> Similar reasoning to the previous patch.
>>> Vittorio
>>>
>>>   libavutil/stereo3d.c | 6 +++---
>>>   libavutil/stereo3d.h | 2 +-
>>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/libavutil/stereo3d.c b/libavutil/stereo3d.c
>>> index 5dc902e909..6e2f9f3ad2 100644
>>> --- a/libavutil/stereo3d.c
>>> +++ b/libavutil/stereo3d.c
>>> @@ -54,9 +54,9 @@ static const char * const stereo3d_type_names[] = {
>>>   [AV_STEREO3D_COLUMNS] = "interleaved columns",
>>>   };
>>>   -const char *av_stereo3d_type_name(unsigned int type)
>>> +const char *av_stereo3d_type_name(enum AVStereo3DType type)
>>>   {
>>> -if (type >= FF_ARRAY_ELEMS(stereo3d_type_names))
>>> +if ((unsigned) type >= FF_ARRAY_ELEMS(stereo3d_type_names))
>>>   return "unknown";
>>> return stereo3d_type_names[type];
>>> @@ -72,5 +72,5 @@ int av_stereo3d_from_name(const char *name)
>>>   return i;
>>>   }
>>>   -return -1;
>>> +return AVERROR(EINVAL);
>>
>>
>> Why EINVAL here but ENOSYS for spherical?
> 
> no reason, i can switch to enonsys

I was thinking EINVAL is more correct for both cases.

> 
>> Also, while changing -1 to AVERROR() isn't a considerable API break as
>> changing NULL on failure to a string on failure, it's still one and really
>> makes me wonder if it's worth doing.
>> While most users probably just check for < 0, in which case both return
>> values would work the same, anyone instead checking explicitly for -1 (a
>> completely valid check as per the documentation, which you for that matter
>> did not update in this patch) will instead break.
> 
> besides the fact that we're in the "break period" after the version
> bump, there should be no reason to explicitly check for -1 so I don't
> think that's a case that it's worth considering
> 

As Anton already stated, there is no such thing as an API breaking
season, only ABI breaking season. You can't change a function signature
or return value/type on a whim if it can break downstream code.
The documentation states the function returns -1 on failure. Explicitly
checking for said value is a correct and completely valid way to check
for failure. This patch would break such downstream code that follows
the documentation to the letter.

Changing unsigned int to enum AVStereo3DType as parameter type in
av_stereo3d_type_name() however should be ok. No API user will be
affected by that.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 2/2] utvideodec: Fix gradient prediction when stride does not match width

[Vittorio Giovara]

On Sat, Apr 22, 2017 at 6:33 AM, Luca Barbato  wrote:
> From: Paul B Mahol 
>
> Signed-off-by: Paul B Mahol 
> Signed-off-by: Luca Barbato 
> ---
>  libavcodec/utvideodec.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
> index 0f58c83..ed6368e 100644
> --- a/libavcodec/utvideodec.c
> +++ b/libavcodec/utvideodec.c
> @@ -686,7 +686,11 @@ static void restore_gradient_planar_il(UtvideoContext 
> *c, uint8_t *src, ptrdiff_
>  C = bsrc[i - 1];
>  bsrc[i] = (A - B + C + bsrc[i]) & 0xFF;
>  }
> -for (i = 0; i < width; i++) {
> +A = bsrc[-stride];
> +B = bsrc[-(1 + stride + stride - width)];
> +C = bsrc[width - 1];
> +bsrc[stride] = (A - B + C + bsrc[stride]) & 0xFF;
> +for (i = 1; i < width; i++) {
>  A = bsrc[i - stride];
>  B = bsrc[i - (1 + stride)];
>  C = bsrc[i - 1 + stride];
> @@ -784,7 +788,11 @@ static void restore_gradient_packed_il(uint8_t *src, int 
> step, ptrdiff_t stride,
>  C = bsrc[i - step];
>  bsrc[i] = (A - B + C + bsrc[i]) & 0xFF;
>  }
> -for (i = 0; i < width * step; i += step) {
> +A = bsrc[-stride];
> +B = bsrc[-(step + stride + stride - width * step)];
> +C = bsrc[width * step - step];
> +bsrc[stride] = (A - B + C + bsrc[stride]) & 0xFF;
> +for (i = step; i < width * step; i += step) {
>  A = bsrc[i - stride];
>  B = bsrc[i - (step + stride)];
>  C = bsrc[i - step + stride];
> --

probably ok
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/2] utvideodec: Fix decoding odd sizes with interlaced video with some formats

[Vittorio Giovara]

On Sat, Apr 22, 2017 at 6:33 AM, Luca Barbato  wrote:
> From: Paul B Mahol 
>
> Signed-off-by: Paul B Mahol 
> Signed-off-by: Luca Barbato 
> ---
>  libavcodec/utvideodec.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/libavcodec/utvideodec.c b/libavcodec/utvideodec.c
> index 910d64b..0f58c83 100644
> --- a/libavcodec/utvideodec.c
> +++ b/libavcodec/utvideodec.c
> @@ -228,6 +228,16 @@ fail:
>  return AVERROR_INVALIDDATA;
>  }
>
> +static int compute_cmask(int plane_no, int interlaced, int pix_fmt)
> +{
> +const int is_luma = (pix_fmt == AV_PIX_FMT_YUV420P) && !plane_no;
> +
> +if (interlaced)
> +return ~(1 + 2 * is_luma);
> +
> +return ~is_luma;
> +}
> +
>  static int decode_plane(UtvideoContext *c, int plane_no,
>  uint8_t *dst, int step, ptrdiff_t stride,
>  int width, int height,
> @@ -238,7 +248,7 @@ static int decode_plane(UtvideoContext *c, int plane_no,
>  VLC vlc;
>  BitstreamContext bc;
>  int prev, fsym;
> -const int cmask = ~(!plane_no && c->avctx->pix_fmt == 
> AV_PIX_FMT_YUV420P);
> +const int cmask = compute_cmask(plane_no, c->interlaced, 
> c->avctx->pix_fmt);
>
>  if (build_huff(src, , )) {
>  av_log(c->avctx, AV_LOG_ERROR, "Cannot build Huffman codes\n");
> --

ok i think
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] avprobe: Handle invalid values for the color description

[Vittorio Giovara]

On Sun, Apr 23, 2017 at 9:23 AM, Luca Barbato  wrote:
> On 4/18/17 7:11 PM, Luca Barbato wrote:
>> On 17/04/2017 18:04, Vittorio Giovara wrote:
>>> On Mon, Apr 17, 2017 at 9:48 AM, Luca Barbato  wrote:
 print_str() cannot print NULL.

 Bug-Id: 1040
 CC: libav-sta...@libav.org
 ---
  avtools/avprobe.c | 36 +++-
  1 file changed, 31 insertions(+), 5 deletions(-)
>>>
>>> I'm not sure about this. First of all these values are not invalid but
>>> simply unknown.
>>
>> So is it ok for you if I use Unknown (something) ?
>>
>
> Ping on that, I'd push this and change it back if there consensus on how
> those functions should behave tilts on having "unknown" returned.
>
> lu
>

go for it
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] [PATCH] pixdesc: Change color property name APIs return type

[Vittorio Giovara]

On Thu, Apr 20, 2017 at 11:26 AM, Vittorio Giovara
 wrote:
> This should make these APIs simpler to use, and less error prone
> in case the caller does not check they are valid, and makes them
> more similar to other naming APIs.
> ---
> This should help in the bug in avprobe found by Luca.
> Sending as RFC since I believe we are allowed to break this API as we
> did the version bump, but I'd like to be sure.
>
> Vittorio

this patch is too controversial, maybe it's better to just drop it
cheers
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [RFC] [PATCH] stereo3d: Update the naming API to be more consistent

[Vittorio Giovara]

On Sun, Apr 23, 2017 at 11:31 PM, James Almer  wrote:
> On 4/20/2017 12:34 PM, Vittorio Giovara wrote:
>>
>> Change input type of the type->str function and return a proper
>> error code for the str->type function.
>> ---
>> Similar reasoning to the previous patch.
>> Vittorio
>>
>>   libavutil/stereo3d.c | 6 +++---
>>   libavutil/stereo3d.h | 2 +-
>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/libavutil/stereo3d.c b/libavutil/stereo3d.c
>> index 5dc902e909..6e2f9f3ad2 100644
>> --- a/libavutil/stereo3d.c
>> +++ b/libavutil/stereo3d.c
>> @@ -54,9 +54,9 @@ static const char * const stereo3d_type_names[] = {
>>   [AV_STEREO3D_COLUMNS] = "interleaved columns",
>>   };
>>   -const char *av_stereo3d_type_name(unsigned int type)
>> +const char *av_stereo3d_type_name(enum AVStereo3DType type)
>>   {
>> -if (type >= FF_ARRAY_ELEMS(stereo3d_type_names))
>> +if ((unsigned) type >= FF_ARRAY_ELEMS(stereo3d_type_names))
>>   return "unknown";
>> return stereo3d_type_names[type];
>> @@ -72,5 +72,5 @@ int av_stereo3d_from_name(const char *name)
>>   return i;
>>   }
>>   -return -1;
>> +return AVERROR(EINVAL);
>
>
> Why EINVAL here but ENOSYS for spherical?

no reason, i can switch to enonsys

> Also, while changing -1 to AVERROR() isn't a considerable API break as
> changing NULL on failure to a string on failure, it's still one and really
> makes me wonder if it's worth doing.
> While most users probably just check for < 0, in which case both return
> values would work the same, anyone instead checking explicitly for -1 (a
> completely valid check as per the documentation, which you for that matter
> did not update in this patch) will instead break.

besides the fact that we're in the "break period" after the version
bump, there should be no reason to explicitly check for -1 so I don't
think that's a case that it's worth considering
-- 
Vittorio
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] configure: Fix sem_timedwait probe

[Diego Biurrun]

On Sun, Apr 23, 2017 at 02:14:32PM +, Luca Barbato wrote:
> It requires pthreads.

configure: Add missing pthreads extralibs to sem_timedwait check

> --- a/configure
> +++ b/configure
> @@ -4693,7 +4693,7 @@ if ! disabled pthreads && ! enabled w32threads; then
>  
>  enabled pthreads &&
> -check_builtin sem_timedwait semaphore.h "sem_t *s; sem_init(s,0,0); 
> sem_timedwait(s,0); sem_destroy(s)"
> +check_builtin sem_timedwait semaphore.h "sem_t *s; sem_init(s,0,0); 
> sem_timedwait(s,0); sem_destroy(s)" $pthreads_extralibs

probably OK

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add ClearVideo decoder

[James Almer]

On 4/24/2017 11:13 AM, Diego Biurrun wrote:
> On Sat, Apr 22, 2017 at 01:05:01PM +0200, Diego Biurrun wrote:
>> --- /dev/null
>> +++ b/libavcodec/clearvideo.c
>> @@ -0,0 +1,387 @@
>> +static int clv_decode_frame(AVCodecContext *avctx, void *data,
>> +int *got_frame, AVPacket *avpkt)
>> +{
>> +if (frame_type & 0x2) {
>> +} else {
>> +/* Only I-frames are supported for now. */
>> +}
> 
> Here I might use avpriv_report_missing_feature() and return
> AVERROR_PATCHWELCOME instead. This would of course result in a lot
> of console spam. Thoughts?
> 
> Diego

Add an did_warn variable to CLVContext, and set it to true after the
warning is printed the first time.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Add ClearVideo decoder

[Diego Biurrun]

On Sat, Apr 22, 2017 at 01:05:01PM +0200, Diego Biurrun wrote:
> --- /dev/null
> +++ b/libavcodec/clearvideo.c
> @@ -0,0 +1,387 @@
> +static int clv_decode_frame(AVCodecContext *avctx, void *data,
> +int *got_frame, AVPacket *avpkt)
> +{
> +if (frame_type & 0x2) {
> +} else {
> +/* Only I-frames are supported for now. */
> +}

Here I might use avpriv_report_missing_feature() and return
AVERROR_PATCHWELCOME instead. This would of course result in a lot
of console spam. Thoughts?

Diego
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel