Re: [libav-devel] [PATCH 00/38] Convert to the new bitstream reader, set 3

2016-05-25 Thread Kostya Shishkov
On Wed, May 25, 2016 at 07:23:11AM +0200, Anton Khirnov wrote:
> Quoting Kostya Shishkov (2016-05-25 07:19:24)
> > On Tue, May 24, 2016 at 04:10:07PM -0400, Ronald S. Bultje wrote:
> > > Hi,
> > > 
> > > On Tue, May 24, 2016 at 3:47 PM, Luca Barbato  wrote:
> > > 
> > > > On 23/05/16 17:01, Ronald S. Bultje wrote:
> > > > > Howdy,
> > > >
> > > > Interesting. I spent a bit of time on it myself.
> > > >
> > > > I run some benchmark using a yuv422 file of the right size from the
> > > > Tim's collection [directly][1] and looped/cut to have a length that
> > > > works fine (1minute and 10 minutes) and I used `perf stat -r 30` on a
> > > > system that surely has a cpu unencumbered by random process on a server,
> > > > so it does not have random quirks like a laptop one.
> > > >
> > > > The benchmark shown that force-inlining bitstream_read_vlc is not
> > > > exactly helpful on the poor constained x86_32, and its implementation
> > > > could spare few branches.
> > > >
> > > > With that change in, looks like the gains for x86_64 get even larger.
> > > >
> > > > I get the dnxhd to be about 3% slower on x86_32 and 20% faster on 
> > > > x86_64.
> > > 
> > > [..]
> > > 
> > > > And with that I guess we are set =)
> > > 
> > > 
> > > But 2 out of 3 are still slower. I can try to look somewhat more into 
> > > this,
> > > but I think that not understanding what makes it slower is fundamentally
> > > flawed.
> > 
> > So you admit you're fundamentally flawed? Sane approach would be to study
> > proper performance tools output, like perf on Linux. Or look at generated
> > assembly and referring to instruction timings and latencies if one wants it
> > hardcore way.
> > 
> > > If we understand why it's slower and we decide that that's OK,
> > > that's an entirely different thing.
> > 
> > Is that royal we or "we, FFmpeg developers"? I'm pretty sure nobody has
> > elected you Libav leader.
> 
> Not this shit again. Can't we have a civilized discussion like normal
> people?

So far there was not much of it.
But don't mind me, I shan't bother you again.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 00/38] Convert to the new bitstream reader, set 3

2016-05-24 Thread Kostya Shishkov
On Tue, May 24, 2016 at 04:10:07PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Tue, May 24, 2016 at 3:47 PM, Luca Barbato  wrote:
> 
> > On 23/05/16 17:01, Ronald S. Bultje wrote:
> > > Howdy,
> >
> > Interesting. I spent a bit of time on it myself.
> >
> > I run some benchmark using a yuv422 file of the right size from the
> > Tim's collection [directly][1] and looped/cut to have a length that
> > works fine (1minute and 10 minutes) and I used `perf stat -r 30` on a
> > system that surely has a cpu unencumbered by random process on a server,
> > so it does not have random quirks like a laptop one.
> >
> > The benchmark shown that force-inlining bitstream_read_vlc is not
> > exactly helpful on the poor constained x86_32, and its implementation
> > could spare few branches.
> >
> > With that change in, looks like the gains for x86_64 get even larger.
> >
> > I get the dnxhd to be about 3% slower on x86_32 and 20% faster on x86_64.
> 
> [..]
> 
> > And with that I guess we are set =)
> 
> 
> But 2 out of 3 are still slower. I can try to look somewhat more into this,
> but I think that not understanding what makes it slower is fundamentally
> flawed.

So you admit you're fundamentally flawed? Sane approach would be to study
proper performance tools output, like perf on Linux. Or look at generated
assembly and referring to instruction timings and latencies if one wants it
hardcore way.

> If we understand why it's slower and we decide that that's OK,
> that's an entirely different thing.

Is that royal we or "we, FFmpeg developers"? I'm pretty sure nobody has
elected you Libav leader.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 3/7] Add the new bitstream reader

2016-05-15 Thread Kostya Shishkov
On Sat, May 14, 2016 at 11:45:03AM +0200, Alexandra Hájková wrote:
> ---
>  libavcodec/bitstream.h | 470 
> +
>  1 file changed, 470 insertions(+)
>  create mode 100644 libavcodec/bitstream.h

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


Re: [libav-devel] [PATCH 3/7] Add the new bitstream reader.

2016-05-09 Thread Kostya Shishkov
On Mon, May 09, 2016 at 03:16:55PM +0200, Luca Barbato wrote:
> On 09/05/16 14:36, Derek Buitenhuis wrote:
> > On 5/9/2016 11:41 AM, Alexandra Hájková wrote:
> >> I was foolowing the ML discussion and used the names the majority agreed 
> >> on,
> >> however I can never satisfy everyone.  I'd suggest  to consider naming
> >> bikesheding to be over.
> > 
> > I must admit though, for people who complain the old names are too 
> > confusing,
> > to call a function "_narrow", which as about as vague as it gets, is pretty
> > ... well... confusing.
> 
> Let me sum up the evolution:
> 
> - The functions had their max range embedded
>   - people used to have the exact range complained
> 
> - Considering their complaint valid, Alexandra accepted to use another
>   set of names.
> 
> - Then somebody suggested to make the 0-63 version the bitstream_read
>   and have the other with a different name.
>   - The suggested name had been _narrow.
> 
> The people complaining the old names being confusing and irrational (me,
> Alexandra, Kostya, Anton) maybe still like to have _32 and _63.

Don't mind me, I'm enjoying the bikeshed.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [RFC] Cineform HD questions

2015-12-30 Thread Kostya Shishkov
On Thu, Dec 31, 2015 at 04:13:50AM +, Kieran Kunhya wrote:
> 
> This patch is the first attempt at getting a working Cineform HD decoder into 
> avcodec
> It supports YUV422P10 files which are the majority of files in the wild
> There are some files not supported such as those from film scanners and some 
> older files which do something unusual with chroma and the transform.
> Also files which are cut awkwardly will infinite loop in the coefficient 
> decoding because of a lack of escape symbol.

A simple condition for checking if you exceeded number of coefficients will be
added anyway (probably by you too).

> The big question is how to organise the coefficients - at the moment all the 
> buffers are hardcoded.
> If anyone has any suggestions about the best way to do this, that would be 
> appreciated.

1) find somebody who can convert transform into lifting scheme so you can do
it inplace (yes, very realistic)
2) have one temporary buffer for merging and store all bands in the same
plane:

   (coeffs) (tmp)(coeffs again)
LL0 | LH0 |  LH1   L0   band0   |  LH1
+-+---  |
HL0 | HH0 |=>  H0 =>|
+-+--   +--
   HL1|  HH1HL1 |  HH1

So you'll have the same stride for input high and low coeffs too.

Captain Obvious' sidekick signing out.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] dcadec: reorganise context data

2015-10-02 Thread Kostya Shishkov
On Fri, Oct 02, 2015 at 02:07:22PM +0200, Vittorio Giovara wrote:
> On Fri, Oct 2, 2015 at 1:48 PM, Alexandra Hájková
>  wrote:
> > place primary audio coding header data into DCAAudioHeader
> > structure and channel related data to DCAChan structure
> > ---
> >  libavcodec/dca.h|  70 -
> >  libavcodec/dcadec.c | 290 
> > 
> >  2 files changed, 201 insertions(+), 159 deletions(-)
> 
> Why is this needed? A little explanation in the commit log would be
> nice (especially for big changes like this one).

"Because Kostya told so" would not make a good explanation :P
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] Screenpresso SPV1 decoder

2015-09-25 Thread Kostya Shishkov
On Fri, Sep 25, 2015 at 04:17:14AM +0200, Vittorio Giovara wrote:
> Signed-off-by: Vittorio Giovara 
> ---
>  Changelog |   1 +
>  configure |   1 +
>  doc/general.texi  |   1 +
>  libavcodec/Makefile   |   1 +
>  libavcodec/allcodecs.c|   1 +
>  libavcodec/avcodec.h  |   1 +
>  libavcodec/codec_desc.c   |   7 ++
>  libavcodec/screenpresso.c | 183 
> ++
>  libavcodec/version.h  |   2 +-
>  libavformat/riff.c|   1 +
>  10 files changed, 198 insertions(+), 1 deletion(-)
>  create mode 100644 libavcodec/screenpresso.c

That's half an hour well spent :)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] Fix typos in header closing comments

2015-09-10 Thread Kostya Shishkov
On Thu, Sep 10, 2015 at 06:53:54PM +0200, Vittorio Giovara wrote:
> On Thu, Sep 10, 2015 at 6:12 PM, Kostya Shishkov
>  wrote:
> > On Thu, Sep 10, 2015 at 05:28:22PM +0200, Vittorio Giovara wrote:
[...]
> >
> > Morale: think more about proper log messages.
> 
> How about "Consistently make sure that every header ends with an
> appropriate comment"?

I vaguely remember there was a special name for such things.
"Sanitise inclusion guards" or something? May Diego be with you.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] Fix typos in header closing comments

2015-09-10 Thread Kostya Shishkov
On Thu, Sep 10, 2015 at 05:28:22PM +0200, Vittorio Giovara wrote:
> ---
>  libavcodec/dct32.h | 2 +-
>  libavcodec/hqxdsp.h| 1 -
>  libavcodec/mpegutils.h | 2 +-
>  libavcodec/pngdsp.h| 2 +-
>  libavcodec/twinvq.h| 2 +-
>  libavcodec/wma_freqs.h | 2 +-
>  6 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/dct32.h b/libavcodec/dct32.h
> index 110338d..8bf6880 100644
> --- a/libavcodec/dct32.h
> +++ b/libavcodec/dct32.h
> @@ -22,4 +22,4 @@
>  void ff_dct32_float(float *dst, const float *src);
>  void ff_dct32_fixed(int *dst, const int *src);
>  
> -#endif
> +#endif /* AVCODEC_DCT32_H */

missing comment isn't a typo IMO

> diff --git a/libavcodec/hqxdsp.h b/libavcodec/hqxdsp.h
> index 163c1f8..2cd2a8e 100644
> --- a/libavcodec/hqxdsp.h
> +++ b/libavcodec/hqxdsp.h
> @@ -37,4 +37,3 @@ typedef struct HQXDSPContext {
>  void ff_hqxdsp_init(HQXDSPContext *c);
>  
>  #endif /* AVCODEC_HQXDSP_H */
> -

This stretches typo definition quite a bit.

> diff --git a/libavcodec/mpegutils.h b/libavcodec/mpegutils.h
> index 5c503fb..9b5a576 100644
> --- a/libavcodec/mpegutils.h
> +++ b/libavcodec/mpegutils.h
> @@ -132,4 +132,4 @@ void ff_draw_horiz_band(AVCodecContext *avctx, AVFrame 
> *cur, AVFrame *last,
>  int y, int h, int picture_structure, int first_field,
>  int low_delay);
>  
> -#endif /* AVCODEC_PICTTYPE_H */
> +#endif /* AVCODEC_MPEGUTILS_H */

That's not a typo but rather a copypaste error.

> diff --git a/libavcodec/pngdsp.h b/libavcodec/pngdsp.h
> index 98d29a8..607fe64 100644
> --- a/libavcodec/pngdsp.h
> +++ b/libavcodec/pngdsp.h
> @@ -37,4 +37,4 @@ typedef struct PNGDSPContext {
>  void ff_pngdsp_init(PNGDSPContext *dsp);
>  void ff_pngdsp_init_x86(PNGDSPContext *dsp);
>  
> -#endif /* AVCDODEC_PNGDSP_H */
> +#endif /* AVCODEC_PNGDSP_H */

This one is typo for a change.

> diff --git a/libavcodec/twinvq.h b/libavcodec/twinvq.h
> index 3148069..3dc0ab4 100644
> --- a/libavcodec/twinvq.h
> +++ b/libavcodec/twinvq.h
> @@ -200,4 +200,4 @@ int ff_twinvq_decode_frame(AVCodecContext *avctx, void 
> *data,
>  int ff_twinvq_decode_close(AVCodecContext *avctx);
>  int ff_twinvq_decode_init(AVCodecContext *avctx);
>  
> -#endif /* AVCODEC_TWINVQ_DATA_H */
> +#endif /* AVCODEC_TWINVQ_H */

Doesn't look like a typo to me.

> diff --git a/libavcodec/wma_freqs.h b/libavcodec/wma_freqs.h
> index 415c83b..d40ab65 100644
> --- a/libavcodec/wma_freqs.h
> +++ b/libavcodec/wma_freqs.h
> @@ -23,4 +23,4 @@
>  
>  extern const uint16_t ff_wma_critical_freqs[25];
>  
> -#endif /* AVCODEC_WMA_FREQS */
> +#endif /* AVCODEC_WMA_FREQS_H */

Ditto.

Morale: think more about proper log messages.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] dxtory: Unify and rework the decoding routines

2015-09-06 Thread Kostya Shishkov
On Sat, Sep 05, 2015 at 03:02:25PM +0200, Luca Barbato wrote:
> Do not make many assumption on the dimension of the slices and just
> try to decode additional lines if there is enough data left.
> 
> Decodes all the samples kindly provided by ultramage.
> ---
> 
> Actually I forgot to squash it with the rest of the intermediate changes
> (and I'm using it to check a corner case in plaid)
> 
>  libavcodec/dxtory.c | 404 
> +---
>  1 file changed, 165 insertions(+), 239 deletions(-)

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


Re: [libav-devel] [PATCH 1/1] fate: make asf-repldata test results identical for all platforms

2015-08-20 Thread Kostya Shishkov
On Thu, Aug 20, 2015 at 02:55:56PM +0200, Anton Khirnov wrote:
> Quoting Janne Grunau (2015-08-20 14:47:45)
> > ---
> >  tests/fate/microsoft.mak|  2 +-
> >  tests/ref/fate/asf-repldata | 28 ++--
> >  2 files changed, 15 insertions(+), 15 deletions(-)
> > 
> > diff --git a/tests/fate/microsoft.mak b/tests/fate/microsoft.mak
> > index 6f83d2e..fad033f 100644
> > --- a/tests/fate/microsoft.mak
> > +++ b/tests/fate/microsoft.mak
> > @@ -63,6 +63,6 @@ FATE_SAMPLES_AVCONV-$(CONFIG_VC1_DECODER) += 
> > $(FATE_VC1-yes)
> >  fate-vc1: $(FATE_VC1-yes)
> >  
> >  FATE_ASF_REPLDATA += fate-asf-repldata
> > -fate-asf-repldata: CMD = framecrc -i $(TARGET_SAMPLES)/asf/bug821-2.asf
> > +fate-asf-repldata: CMD = framecrc -flags +bitexact -idct simple -i 
> > $(TARGET_SAMPLES)/asf/bug821-2.asf
> 
> Since this is supposed to be a demuxer test, perhaps it'd be better to
> just use -c copy.

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


Re: [libav-devel] Disguised VDD reminder

2015-08-09 Thread Kostya Shishkov
On Sat, Aug 08, 2015 at 09:29:23AM +0200, Jean-Baptiste Kempf wrote:
> Hello my libav fellows,
> 
> VideoLAN Dev Days 2015, in Paris, is approaching.
> It's on September 19-20, 2015, in the St Lazare district, in Paris.
> 
> So far, we've registration from numerous people from VLC, libav, FFmpeg,
> x264, x265, KDE, MPlayer, aegisub, handbrake, and many other...
> We've already planned talks about x265, VP*, Daala, VLC.

IIRC you planned a talk about GoToMeeting family, maybe even G2M7 VLC Edition.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] h264: Do not print an error when the buffer has to be refilled

2015-08-07 Thread Kostya Shishkov
On Thu, Aug 06, 2015 at 11:57:53AM +0200, Luca Barbato wrote:
> Partially amends 9469370fb32679352e66826daf77bdd2e6f067b5
> ---
> 
> My fault for testing only with fate =P
> 
>  libavcodec/h264.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
> index bf2ae36..d4cb030 100644
> --- a/libavcodec/h264.c
> +++ b/libavcodec/h264.c
> @@ -1274,11 +1274,8 @@ static int get_avc_nalsize(H264Context *h, const 
> uint8_t *buf,
>  int i, nalsize = 0;
> 
>  if (*buf_index >= buf_size - h->nal_length_size) {
> -av_log(h->avctx, AV_LOG_ERROR,
> -   "AVC: The buffer size %d is too short to read "
> -   "the nal length size %d at the offset %d.\n",
> -   buf_size, h->nal_length_size, *buf_index);
> -return AVERROR_INVALIDDATA;
> +// the end of the buffer is reached, refill it.
> +return AVERROR(EAGAIN);
>  }
> 
>  for (i = 0; i < h->nal_length_size; i++)
> --

Since it's obvious why, OK.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 7/7] dnxhddata: Add tables for missing DNx100 profiles

2015-07-30 Thread Kostya Shishkov
On Thu, Jul 30, 2015 at 03:55:39PM +0100, Vittorio Giovara wrote:
> 1440x1080@8 progressive (1259) and interlaced (1260).
> 
> Signed-off-by: Vittorio Giovara 
> ---
> Note that profile 1260 suffers from a dct overflow, which corrupts the
> decoded frame entirely. The bug is unrelated to the tables, so a separate
> fix might be applied afterwards.

Sure, when someone submits it (also the whole set LGTM on a quick glance but
that's irrelevant).
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 0/20] removal of deprecated features

2015-07-29 Thread Kostya Shishkov
On Wed, Jul 29, 2015 at 09:07:53AM +0200, Anton Khirnov wrote:
> Quoting Kostya Shishkov (2015-07-29 08:59:42)
> > On Wed, Jul 29, 2015 at 08:44:48AM +0200, Anton Khirnov wrote:
> > > Quoting Kostya Shishkov (2015-07-28 15:54:32)
> > > > On Tue, Jul 28, 2015 at 02:36:16PM +0100, Vittorio Giovara wrote:
> > > > > This set contains the removal of all deprecated features marked as
> > > > > such until 2012/early 2013. This was announced several times in the
> > > > > past months and agreed at several meetings (since fosdem and recently
> > > > > at the sprint).
> > > > > 
> > > > > With more than two year span, downstream users should have had enough
> > > > > time to update their API usage (or comment otherwise).
> > > > 
> > > > I'd say swscale in general is a deprecated feature or should be one. 
> > > > And it's
> > > > your fault it's still not been replaced.
> > > 
> > > I strongly suspect you could have written mountains of SIMD for avscale
> > > in the time it takes you to write these mails =p
> > 
> > Maybe, there's just no point doing all that alone with no clear goal.
> 
> Here's a clear goal:
> 1) make avscale feature complete
> 2) deprecate swscale
> 3) ???
> 4) profit

So far I like only step #3, avscale might happen but not in Libav.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 0/20] removal of deprecated features

2015-07-29 Thread Kostya Shishkov
On Wed, Jul 29, 2015 at 08:44:48AM +0200, Anton Khirnov wrote:
> Quoting Kostya Shishkov (2015-07-28 15:54:32)
> > On Tue, Jul 28, 2015 at 02:36:16PM +0100, Vittorio Giovara wrote:
> > > This set contains the removal of all deprecated features marked as
> > > such until 2012/early 2013. This was announced several times in the
> > > past months and agreed at several meetings (since fosdem and recently
> > > at the sprint).
> > > 
> > > With more than two year span, downstream users should have had enough
> > > time to update their API usage (or comment otherwise).
> > 
> > I'd say swscale in general is a deprecated feature or should be one. And 
> > it's
> > your fault it's still not been replaced.
> 
> I strongly suspect you could have written mountains of SIMD for avscale
> in the time it takes you to write these mails =p

Maybe, there's just no point doing all that alone with no clear goal.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] Add libkvazaar HECV encoder

2015-07-28 Thread Kostya Shishkov
On Tue, Jul 28, 2015 at 03:23:27PM +0100, Vittorio Giovara wrote:

typo in the log message
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 0/20] removal of deprecated features

2015-07-28 Thread Kostya Shishkov
On Tue, Jul 28, 2015 at 02:36:16PM +0100, Vittorio Giovara wrote:
> This set contains the removal of all deprecated features marked as
> such until 2012/early 2013. This was announced several times in the
> past months and agreed at several meetings (since fosdem and recently
> at the sprint).
> 
> With more than two year span, downstream users should have had enough
> time to update their API usage (or comment otherwise).

I'd say swscale in general is a deprecated feature or should be one. And it's
your fault it's still not been replaced.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/6] nut: Support HEVC

2015-07-27 Thread Kostya Shishkov
On Mon, Jul 27, 2015 at 04:24:12PM +0200, Anton Khirnov wrote:
> Quoting Luca Barbato (2015-07-25 15:26:27)
> > ---
> >  libavformat/nut.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/libavformat/nut.c b/libavformat/nut.c
> > index 828d9ca..beb27fc 100644
> > --- a/libavformat/nut.c
> > +++ b/libavformat/nut.c
> > @@ -40,6 +40,7 @@ const AVCodecTag ff_nut_data_tags[] = {
> >  
> >  const AVCodecTag ff_nut_video_tags[] = {
> >  { AV_CODEC_ID_VP9,  MKTAG('V', 'P', '9', '0') },
> > +{ AV_CODEC_ID_HEVC, MKTAG('H', 'E', 'V', 'C') },
> >  { AV_CODEC_ID_RAWVIDEO, MKTAG('R', 'G', 'B',  15) },
> >  { AV_CODEC_ID_RAWVIDEO, MKTAG('B', 'G', 'R',  15) },
> >  { AV_CODEC_ID_RAWVIDEO, MKTAG('R', 'G', 'B',  16) },
> > -- 
> > 2.3.2
> > 
> 
> Is it properly defined what this will do exactly? What
> bitstream/extradata format is used and such?

Yes, just ask Luca for proper NUT specs.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [FFmpeg-devel] Remote participation options for IETF session on MKV/FFV1 at July 22 @ 9 CEST

2015-07-22 Thread Kostya Shishkov
On Tue, Jul 21, 2015 at 11:12:13PM +0200, Jerome Martinez wrote:
> Le 21/07/2015 20:03, Ronald S. Bultje a écrit :
> >+1. I can't stress how important this is. In addition, the spec
> >should be the master, not any one implementation (because then the
> >bugs in that one implementation will "be" the spec, regardless of
> >what the bug is).
> 
> In theory, spec should be the reference, I totally agree.
> In practice, both Matroska and FFV1 formal specifications show up
> after these formats are widely used, without relying on any
> specification. So saying that the specification is the reference
> does not make a lot of sense here.

That's the approach used mostly for audio codecs. Video codecs are supposed to
follow specification instead (if there's one).

> I argue for:
> - previous and current versions: specifications are more an official
> state of the art and we try to have a specification the most
> "compatible" with most used tools. If 2 tools are incompatible, we
> discuss with developers case by case about the best method for
> fixing the incompatibility and we write the decision as part of the
> specification. Example of specification which takes care of
> compatibility with files in the wild: "O is a reserved value.
> Encoders MUST NOT store bits_per_raw_sample = 0. Decoders SHOULD
> accept and interpret bits_per_raw_sample = 0 as 8."

Most specifications I've seen concentrate on decoding point of view, encoders
are supposed to produce compliant bitstream but it's really up to decoder to
decide what to do with a non-compliant input (try to decode anyway/error
out/launch tetris).

> - next version: the spec is the master, not any one implementation,
> and we have 2-3 independent implementations ready *before* the
> formal release of the specification.

Even one truly independent implementation is enough IMO. Even better if one
party implements an encoder and another party implements a decoder from the
specification and they test how well those implementations work with each
other.

> FYI, we are on the way of having a completely independent FFV1
> parser/decoder in libmediainfo and a complete Matroska and FFV1
> conformance checker tool, without relying on other libraries (e.g.
> ffmpeg, libav, libmatroska) so we hope to catch any issue in the
> specs and/or files created by other tools before the formal release
> of specifications.

That's the best way indeed.

> Please comment.

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


Re: [libav-devel] [FFmpeg-devel] Remote participation options for IETF session on MKV/FFV1 at July 22 @ 9 CEST

2015-07-22 Thread Kostya Shishkov
On Tue, Jul 21, 2015 at 04:10:23PM -0400, Dave Rice wrote:
> 
> > On Jul 21, 2015, at 2:59 PM, Michael Niedermayer  
> > wrote:
> > 
> > On Tue, Jul 21, 2015 at 02:03:16PM -0400, Ronald S. Bultje wrote:
> >> Hi,
> >> 
> >> On Tue, Jul 21, 2015 at 12:58 PM, Kostya Shishkov 
> >>  >>> wrote:
> >> 
> >>> On Tue, Jul 21, 2015 at 11:52:55AM -0400, Dave Rice wrote:
> >>>> Hi all,
> >>> [...]
> >>>> The FFV1 specification work may also be reviewed at github [5] with
> >>> recent rendering in HTML [6] and PDF [7] available. To participate in the
> >>> current standardization efforts of FFV1 please visit the ffmpeg-devel
> >>> mailing list [8] or the #ffmpeg-devel [8] IRC channel on freenode.
> >>> 
> >>> I'd suggest that any standardisation includes not only "specification" but
> >>> also an independent implementation - it helps to figure out what's wrong
> >>> with
> >>> the specification and maybe gives a small standalone library instead of
> >>> something spread out on half a dozen files in a large software project.
> >> 
> >> 
> >> +1. I can't stress how important this is. In addition, the spec should be
> >> the master, not any one implementation (because then the bugs in that one
> >> implementation will "be" the spec, regardless of what the bug is).
> >> 
> >> Thank you Kostya.
> > 
> > that makes sense for future versions but not for past ones
> > There is a large number of existing ffv1 files out there.
> > Now most likely spec and implementation match anyway but assuming they
> > do not match for version 1 then there are 2 options
> > A: change the spec for version 1
> > B: change the implementation for version 1
> 
> IMHO I'd propose option A in most cases. IIRC, the implementation of version 
> 1 of FFV1 came before the specification for FFV1 was in development and the 
> goal of the specification is to properly document the implementation. Also 
> the implementation of version 1 had some sort of event (the removal of the 
> experimental flag 
> http://git.videolan.org/?p=ffmpeg.git;a=commit;h=b548f2b91b701e1235608ac882ea6df915167c7e)
>  that signified a form of official status. The specification of version 1 
> started years later (2012?) and has been in gradual development with no event 
> or commit that has signified that the specification is official. IMO the 
> implementation of version 1 is complete and in use and the specification 
> continues to be under development to define the implementation. At some 
> point, hopefully after enough eyes verify that the implementation and the 
> specification match, the specification should be marked as official (ideally 
> through an open standards organization). Still it seems wise to,
  a
>  s Opus did, declare what happens if a mismatch between the spec and the 
> implementation is discovered at a later point.

C: go for version 2 standardised and implemented sanely (or FFv1/A :)

I'd like to point out that creating a new implementation when you know what
you do is not hard in this case, modifying current decoder to support such
profile/modification should be trivial too.

> > If now all implementations match and just the spec mismatches then
> > 
> > If the spec is changed it would probably affect noone
> 
> Presently MediaArea is tasked with developing a conformance checker with 
> FFV1. Jerome has attempted this work solely off of the specification and not 
> the implementation, but the specification is not sufficiently 
> self-descriptive to support developing another implementation without a 
> review of the existing implementation. This issue is why we had initially 
> proposed a standardization project.

Welcome to my world. I've seen enough incomplete specifications,
specifications that simply contained source code (VP8...)
or specifications that told you to look at the source code (DTS tables...).
This one has a good chance to become one of those too.
 
> > If the implementation is changed then suddenly there are 2
> > interpretations of what version 1 means and possibly no easy way to
> > find out if a existing file used the old or new one (as both would
> > claim to be version 1). That would be really bad, especially for a
> > lossless format.
> 
> Agreed.

Lossless format? With CRCs embedded too? It does not sound that hard to test
whether decoder interprets input correctly. And encoders abusing or violating
standard are quite common for widespread formats anyway.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] Remote participation options for IETF session on MKV/FFV1 at July 22 @ 9 CEST

2015-07-21 Thread Kostya Shishkov
On Tue, Jul 21, 2015 at 11:52:55AM -0400, Dave Rice wrote:
> Hi all,
[...] 
> The FFV1 specification work may also be reviewed at github [5] with recent 
> rendering in HTML [6] and PDF [7] available. To participate in the current 
> standardization efforts of FFV1 please visit the ffmpeg-devel mailing list 
> [8] or the #ffmpeg-devel [8] IRC channel on freenode.

I'd suggest that any standardisation includes not only "specification" but
also an independent implementation - it helps to figure out what's wrong with
the specification and maybe gives a small standalone library instead of
something spread out on half a dozen files in a large software project.

Sincerely yours, the guy who is not welcome at ffmpeg-devel or #ffmpeg-devel
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] asfdec: do not read replicated data when their length is 0

2015-07-16 Thread Kostya Shishkov
On Thu, Jul 16, 2015 at 01:55:47PM +0200, Luca Barbato wrote:
> On 16/07/15 12:14, Alexandra Hájková wrote:
> > ---
> >  libavformat/asfdec.c | 24 +---
> >  1 file changed, 13 insertions(+), 11 deletions(-)
> > 
> > diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
> > index 7458e19..7146dcc 100644
> > --- a/libavformat/asfdec.c
> > +++ b/libavformat/asfdec.c
> > @@ -1123,17 +1123,19 @@ static int 
> > asf_read_multiple_payload(AVFormatContext *s, AVPacket *pkt,
> >  if ((ret = asf_read_subpayload(s, pkt, 1)) < 0)
> >  return ret;
> >  } else {
> > -if (!asf_pkt->data_size) {
> > -asf_pkt->data_size = asf_pkt->size_left = avio_rl32(pb); // 
> > read media object size
> > -if (asf_pkt->data_size <= 0)
> > -return AVERROR_EOF;
> > -if ((ret = av_new_packet(&asf_pkt->avpkt, asf_pkt->data_size)) 
> > < 0)
> > -return ret;
> > -} else
> > -avio_skip(pb, 4); // reading of media object size is already 
> > done
> > -asf_pkt->dts = avio_rl32(pb); // read presentation time
> > -if ((asf->rep_data_len - 8) > 0)
> > -avio_skip(pb, asf->rep_data_len - 8); // skip replicated data
> > +if (asf->rep_data_len) {
> > +if (!asf_pkt->data_size) {
> > +asf_pkt->data_size = asf_pkt->size_left = avio_rl32(pb); 
> > // read media object size
> > +if (asf_pkt->data_size <= 0)
> > +return AVERROR_EOF;
> > +if ((ret = av_new_packet(&asf_pkt->avpkt, 
> > asf_pkt->data_size)) < 0)
> > +return ret;
> > +} else
> > +avio_skip(pb, 4); // reading of media object size is 
> > already done
> > +asf_pkt->dts = avio_rl32(pb); // read presentation time
> > +if (asf->rep_data_len && ((asf->rep_data_len - 8) > 0))
> > +avio_skip(pb, asf->rep_data_len - 8); // skip replicated 
> > data
> > +}
> >  pay_len = avio_rl16(pb); // payload length should be WORD
> >  if (pay_len > asf->packet_size) {
> >  av_log(s, AV_LOG_ERROR,
> > 
> 
> Would be probably a good idea spin that block in a stand alone function.

Also has anyone noticed that rep_data_len is unsigned and thus
(asf->rep_data_len - 8) > 0 might behave funny?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] mp3: Make the seek more robust

2015-07-11 Thread Kostya Shishkov
On Sat, Jul 11, 2015 at 09:01:49PM +0200, Luca Barbato wrote:
> On 11/07/15 17:41, Luca Barbato wrote:
> > Try to parse up to 4 packets to find the closest packet.
> > 
> > Reported-By: jan.schlue...@ofai.at
> > ---
> >  libavformat/mp3dec.c | 78 
> > 
> >  1 file changed, 67 insertions(+), 11 deletions(-)
> > 
> 
> Vittorio and kropping aren't against it, or so he said.

Is it a good time to remind y'all are the same guy?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] riffdec: error out on negative bit rate

2015-07-11 Thread Kostya Shishkov
On Sat, Jul 11, 2015 at 11:35:44AM +0200, Luca Barbato wrote:
> On 11/07/15 00:39, Andreas Cadhalpun wrote:
> > Signed-off-by: Andreas Cadhalpun 
> > ---
> >  libavformat/riffdec.c | 5 +
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/libavformat/riffdec.c b/libavformat/riffdec.c
> > index eebd8ed..be55699 100644
> > --- a/libavformat/riffdec.c
> > +++ b/libavformat/riffdec.c
> > @@ -106,6 +106,11 @@ int ff_get_wav_header(AVIOContext *pb, AVCodecContext 
> > *codec, int size, int big_
> >  codec->bit_rate= avio_rb32(pb) * 8;
> >  codec->block_align = avio_rb16(pb);
> >  }
> > +if (codec->bit_rate < 0) {
> > +av_log(NULL, AV_LOG_ERROR,
> > +   "Invalid bit rate: %d\n", codec->bit_rate);
> > +return AVERROR_INVALIDDATA;
> > +}
> 
> I'm quite sure some codecs do not have a fixed bit_rate while some other
> (like the g726 below) do use it.
> 
> So I'd expect this change to break on some semi-valid files and prevent
> something horrid in other.

I know how to prevent it in much easier way - RTFM, see that byte rate in
WAVEFORMAT is an unsigned number and get back to sleep.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] hq_hqa: Fix decoding when INFO section is absent

2015-07-10 Thread Kostya Shishkov
On Fri, Jul 10, 2015 at 09:09:44PM +0200, Luca Barbato wrote:
> On 10/07/15 16:52, Vittorio Giovara wrote:
> > ---
> >  libavcodec/hq_hqa.c | 6 --
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavcodec/hq_hqa.c b/libavcodec/hq_hqa.c
> > index ae378e6..4871c59 100644
> > --- a/libavcodec/hq_hqa.c
> > +++ b/libavcodec/hq_hqa.c
> > @@ -310,9 +310,11 @@ static int hq_hqa_decode_frame(AVCodecContext *avctx, 
> > void *data,
> >  return AVERROR_INVALIDDATA;
> >  }
> >  
> > -info_tag = bytestream2_get_le32(&ctx->gbc);
> > +info_tag = bytestream2_peek_le32(&ctx->gbc);
> >  if (info_tag == MKTAG('I', 'N', 'F', 'O')) {
> > -int info_size = bytestream2_get_le32(&ctx->gbc);
> > +int info_size;
> > +bytestream2_skip(&ctx->gbc, 4);
> > +info_size = bytestream2_get_le32(&ctx->gbc);
> >  if (bytestream2_get_bytes_left(&ctx->gbc) < info_size) {
> >  av_log(avctx, AV_LOG_ERROR, "Invalid INFO size (%d).\n", 
> > info_size);
> >  return AVERROR_INVALIDDATA;
> > 
> 
> Ok.

Doesn't that apply to HQX and CLLC as well?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] libvcx: Add library header

2015-07-09 Thread Kostya Shishkov
On Thu, Jul 09, 2015 at 03:02:48PM +0200, Luca Barbato wrote:
> On 09/07/15 14:49, Kostya Shishkov wrote:
> > On Thu, Jul 09, 2015 at 02:45:04PM +0200, Luca Barbato wrote:
> >> On 09/07/15 14:35, Vittorio Giovara wrote:
> >>> Unbreak 'make checkheaders'.
> >>> ---
> >>>  libavcodec/libvpx.h | 2 ++
> >>>  libavcodec/lzf.h| 1 -
> >>>  2 files changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>
> >> Unbreak the "libvcx" subject line while at it =) (and drop the lzf)
> > 
> > s/drop/move to lavu/
> 
> I'd rather keep the decompressors/compressors in avcodec initially since
> the api isn't set at all =) (and currently they use bytestream).

Anyway, with LZF decompressor one can hope Derek will have less reasons not to
write IgCodec decoder (but I guess he still won't write it no matter what).
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] libvcx: Add library header

2015-07-09 Thread Kostya Shishkov
On Thu, Jul 09, 2015 at 02:45:04PM +0200, Luca Barbato wrote:
> On 09/07/15 14:35, Vittorio Giovara wrote:
> > Unbreak 'make checkheaders'.
> > ---
> >  libavcodec/libvpx.h | 2 ++
> >  libavcodec/lzf.h| 1 -
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> > 
> 
> Unbreak the "libvcx" subject line while at it =) (and drop the lzf)

s/drop/move to lavu/
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel



Re: [libav-devel] [PATCH] libvcx: Add library header

2015-07-09 Thread Kostya Shishkov
On Thu, Jul 09, 2015 at 01:35:01PM +0100, Vittorio Giovara wrote:
> Unbreak 'make checkheaders'.
> ---
>  libavcodec/libvpx.h | 2 ++
>  libavcodec/lzf.h| 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/libvpx.h b/libavcodec/libvpx.h
> index b990b76..b437f37 100644
> --- a/libavcodec/libvpx.h
> +++ b/libavcodec/libvpx.h
> @@ -21,6 +21,8 @@
>  #ifndef AVCODEC_LIBVPX_H
>  #define AVCODEC_LIBVPX_H
>  
> +#include 
> +
>  #include "avcodec.h"
>  
>  enum AVPixelFormat ff_vpx_imgfmt_to_pixfmt(vpx_img_fmt_t img);
> diff --git a/libavcodec/lzf.h b/libavcodec/lzf.h
> index ff05e9c..4951f25 100644
> --- a/libavcodec/lzf.h
> +++ b/libavcodec/lzf.h
> @@ -27,4 +27,3 @@
>  int ff_lzf_uncompress(GetByteContext *gb, uint8_t **buf, int64_t *size);
>  
>  #endif /* AVCODEC_LZF_H */
> -
> -- 

The second part of this patch looks infintely much better than the first one
even if a bit unrelated.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] aic: Improve logging

2015-06-15 Thread Kostya Shishkov
On Mon, Jun 15, 2015 at 05:52:08PM +0100, Vittorio Giovara wrote:
> ---

http://www.oxforddictionaries.com/definition/english/logging
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] lavf: Replace the ASF demuxer

2015-05-16 Thread Kostya Shishkov
On Sat, May 16, 2015 at 04:54:42PM +0200, Alexandra Hájková wrote:
> The old one is the result of the reverse engineering and guesswork.
> The new one has been written following the now-available specification.
> 
> This work is part of Outreach Program for Women Summer 2014 activities
> for the Libav project.
> 
> The fate references had to be changed because the old demuxer truncates
> the last frame in some cases, the new one handles it properly.
> The seek-test reference is changed because seeking works differently
> in the new demuxer. When seeking, the packet is not read from the stream
> directly, but it is rather constructed by the demuxer. That is why
> position is -1 now in the reference.
> ---

LGTM (you've been asking for such comment since ages after all)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] isom: Add X-Com Radvision FourCC

2015-03-16 Thread Kostya Shishkov
On Mon, Mar 16, 2015 at 11:47:11AM +0100, Luca Barbato wrote:
> On 16/03/15 11:42, Vittorio Giovara wrote:
> >From: Paul B Mahol 
> >
> >Signed-off-by: Paul B Mahol 
> >---
> >  libavformat/isom.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> >diff --git a/libavformat/isom.c b/libavformat/isom.c
> >index 68ddd32..fcd696a 100644
> >--- a/libavformat/isom.c
> >+++ b/libavformat/isom.c
> >@@ -173,6 +173,7 @@ const AVCodecTag ff_codec_movvideo_tags[] = {
> >  { AV_CODEC_ID_H264, MKTAG('a', 'i', '1', '6') }, /* AVC-Intra 100M 
> > 1080i60 */
> >  { AV_CODEC_ID_H264, MKTAG('A', 'V', 'i', 'n') }, /* AVC-Intra with 
> > implicit SPS/PPS */
> >  { AV_CODEC_ID_H264, MKTAG('a', 'i', 'v', 'x') }, /* XAVC 4:2:2 10bit */
> >+{ AV_CODEC_ID_H264, MKTAG('r', 'v', '6', '4') }, /* X-Com Radvision */
> >
> >  { AV_CODEC_ID_MPEG1VIDEO, MKTAG('m', '1', 'v', ' ') },
> >  { AV_CODEC_ID_MPEG1VIDEO, MKTAG('m', '1', 'v', '1') }, /* Apple MPEG-1 
> > Camcorder */
> >
> 
> Probably fine.

Until there's RealVideo 6.4 and some idiot tries to put it into MOV at least.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/2] dca: Support for XLL (lossless extension)

2015-02-24 Thread Kostya Shishkov
On Tue, Feb 24, 2015 at 02:39:04PM +0100, Diego Biurrun wrote:
> On Tue, Feb 24, 2015 at 02:22:12PM +0100, Anton Khirnov wrote:
> > Quoting Diego Biurrun (2015-02-24 14:01:22)
> > > On Tue, Feb 24, 2015 at 12:28:45PM +0100, Anton Khirnov wrote:
> > > > Quoting Diego Biurrun (2015-02-24 12:07:51)
> > > > > From: Niels Möller 
> > > > > 
> > > > > Cleanup and reengineering by Diego Biurrun.
> > > > > 
> > > > > Signed-off-by: Diego Biurrun 
> > > > > ---
> > > > > 
> > > > > The XLL extension now lives in a separate file.
> > > > > I added a version bump, documentation and changelog entry.
> > > > > Minor cosmetics.
> > > > 
> > > > And is it lossless yet?
> > > 
> > > No.  We still need a fixed-point transform for that.  Should I mark
> > > the code in some way as (sort of) incomplete?
> > > 
> > 
> > Yes, it should certainly be marked in a visible way, and the commit
> > message should also mention that.
> > 
> > Alternatively, you could write the transform yourself. Kostya tells me
> > it's not hard =p
> 
> Kostya is going to rip my head off (deservedly, even) if this thing gets
> delayed much longer ...

Don't worry, there are other things you've been delaying for too long.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] Canopus HQX decoder

2015-02-05 Thread Kostya Shishkov
On Thu, Feb 05, 2015 at 04:23:10PM +, Derek Buitenhuis wrote:
> On 2/5/2015 1:50 PM, Vittorio Giovara wrote:
> > Based on work by Kostya Shishkov .
> > ---
> 
> It looks fairly straightforward to me. OK.
> 
> I am sure Kostya is ashamed of me right now. Luckily I have an
> excuse to avoid work (VC-5 support) in that no samples exist outside
> of test vectors.

Pity I could not use the same excuse with VC-1 though no samples existed
outside of test vectors. But with minor modifications the same decoder could
be used to decode WMV3.

Good for you VC-5 is unrelated to Cineform codec though.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


[libav-devel] So Long and Thanks for All the Fish

2014-08-29 Thread Kostya Shishkov
I've been first FFmpeg then Libav contributor for ten years and mostly
it was fun experience but in the last three years it became less and less
so.  I don't want to talk about the reasons why, you know them well enough.
It was my intent to bid farewell to the project maybe this autumn, but
today seems a good day to do that as well.
Hereby I declare that I'm no more an active Libav developer and I'm not
going to participate in Libav development, including codec development and
bugfixing.  If you have such requests - direct them to someone else. I'd
rather spend time on some game projects or something else.

P.S. It was still fun working on multimedia and helping other peoples.
And I value the fact I could met at least some Libav developers and spend some
time with them, maybe we'll meet again somewhere.
P.P.S. Please do not comment this letter, there's no need for that and it
won't change anything.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/1] rv34: ido not leave the context half initilized on errors

2014-08-21 Thread Kostya Shishkov
On Thu, Aug 21, 2014 at 01:53:26PM +0200, Janne Grunau wrote:
> Hi,
> 
> this is a little more thourough and less ridicolous patch as
> alternative for your WIP patch 1. I think this is safer alternative.
> There are probably similar frame threading decoding dead locks in other
> mpegenccontext based decoders. So your pthread_frame fix is probably
> still a good idea.
> For release branches I prefer this though.
> 
> Janne
> 
> ---8<---
> 
> MpegEncContext based decoders are only fully initialized after the
> ff_thread_get_buffer() call. If an decoding error occurs before the
> first ff_thread_get_buffer() call the decoder is left in state which
> causes dead locks in frame threaded decoding.
> 
> CC: libav-sta...@libav.org
> ---
>  libavcodec/rv34.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/libavcodec/rv34.c b/libavcodec/rv34.c
> index 0c36348..4dd154c 100644
> --- a/libavcodec/rv34.c
> +++ b/libavcodec/rv34.c
> @@ -1635,22 +1635,24 @@ int ff_rv34_decode_frame(AVCodecContext *avctx,
>  }else
>  slice_count = avctx->slice_count;
>  
> +#define ERR_INVALIDDATA(msg)  \
> +av_log(avctx, AV_LOG_ERROR, msg); \
> +if (!s->linesize) s->context_initialized = 0; \
> +return AVERROR_INVALIDDATA;
> +
>  //parse first slice header to check whether this frame can be decoded
>  if(get_slice_offset(avctx, slices_hdr, 0) < 0 ||
> get_slice_offset(avctx, slices_hdr, 0) > buf_size){
> -av_log(avctx, AV_LOG_ERROR, "Slice offset is invalid\n");
> -return AVERROR_INVALIDDATA;
> +ERR_INVALIDDATA("Slice offset is invalid\n");
>  }
>  init_get_bits(&s->gb, buf+get_slice_offset(avctx, slices_hdr, 0), 
> (buf_size-get_slice_offset(avctx, slices_hdr, 0))*8);
>  if(r->parse_slice_header(r, &r->s.gb, &si) < 0 || si.start){
> -av_log(avctx, AV_LOG_ERROR, "First slice header is incorrect\n");
> -return AVERROR_INVALIDDATA;
> +ERR_INVALIDDATA("First slice header is incorrect\n");
>  }
>  if ((!s->last_picture_ptr || !s->last_picture_ptr->f->data[0]) &&
>  si.type == AV_PICTURE_TYPE_B) {
> -av_log(avctx, AV_LOG_ERROR, "Invalid decoder state: B-frame without "
> -   "reference data.\n");
> -return AVERROR_INVALIDDATA;
> +ERR_INVALIDDATA("Invalid decoder state: B-frame without reference "
> +"data.\n");
>  }
>  if(   (avctx->skip_frame >= AVDISCARD_NONREF && 
> si.type==AV_PICTURE_TYPE_B)
> || (avctx->skip_frame >= AVDISCARD_NONKEY && 
> si.type!=AV_PICTURE_TYPE_I)
> -- 

I hope it doesn't leak, in general fine with me.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] proresenc: warn/reject on incorrect profile

2014-08-18 Thread Kostya Shishkov
On Mon, Aug 18, 2014 at 06:49:34PM +0200, Christophe Gisquet wrote:
> 2014-08-18 18:35 GMT+02:00 Christophe Gisquet :
> >> As much as I read
> >> https://documentation.apple.com/en/finalcutpro/professionalformatsandworkflows/index.html#chapter=10%26section=2%26tasks=true
> >> it seems that alpha is not mandatory there.
> >
> > OK, so this hunk would not be required. The first intent was that if
> > the user selects the  profile, he probably wanted some alpha, and
> > he somehow got it wrong. I'm also waiting for a user to report if 
> > content without alpha has issues or not.
> 
> Final Cut Pro works with "-pix_fmt yuv444p10le -profile " so the
> second reason is not valid. If you want I can drop it.

As I understand it, the patch should only error out on non-experimental
compliance level  when clueless users tries to encode source with alpha
and forgot to select  profile. The rest is not needed.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] proresenc: warn/reject on incorrect profile

2014-08-18 Thread Kostya Shishkov
On Mon, Aug 18, 2014 at 05:50:36PM +0200, Christophe Gisquet wrote:
> Hi,
> 
> 2014-08-18 16:50 GMT+02:00 Kostya Shishkov :
> > On Mon, Aug 18, 2014 at 04:22:28PM +0200, Christophe Gisquet wrote:
> >> This fixes the following situations:
> >> - Reject encoding when profile selected encodes alpha, but the pixel format
> >>   does not have an alpha channel;
> >
> > That looks very wrong - i.e. by default we have alpha_bits=16 and nothing
> > should stop user from feeding YUV444(no alpha) for  profile. Currently 
> > it
> > simply won't attempt to encode alpha, with your change it will error out.
> 
> I misinterpreted your sentence "maybe not encode alpha for non-
> profile and refuse YUV444 for the same too?"
> Also, I was not willing to try my luck on how commercial software
> would behave. But OK.
> 
> >> - Warn if the pixel format has an alpha channel but the profile does not
> >>   encode it;
> >> - Do not encode alpha if the profile does not specify it.
> >
> > In general I'd reformulate it differently - "allow alpha plane encoding only
> > with  profile because of original software yada yada blah"
> [...]
> > Maybe add strictness checking level so that people can force alpha encoding
> > with other profiles? Original decoder will ignore it, ours won't.
> 
> The attached patch now changes behavior only for > normal strictness, where:
> - alpha is skipped if profile does not allow alpha, and user is warned;
> - if profile allows alpha but content doesn't have alpha, user is warned.
> 
> The later may actually be an error, but I have no idea how random
> software will behave.
> 
> -- 
> Christophe

> From ed8d88b0ea135205b09accca7f443b28689043f8 Mon Sep 17 00:00:00 2001
> From: Christophe Gisquet 
> Date: Mon, 18 Aug 2014 11:27:50 +0200
> Subject: [PATCH] proresenc: skip/warn on incorrect profile
> 
> This does, if strictness level is high enough:
> - Skip encoding of alpha when profile does not allow it;
> - Warn about profile allowing alpha when content does not have alpha.
> 
> Some software such as After Effects and Final Cut Pro entirely skip
> alpha channel decoding if the profile is not .
> ---
>  libavcodec/proresenc.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/proresenc.c b/libavcodec/proresenc.c
> index bbb414e..6905987 100644
> --- a/libavcodec/proresenc.c
> +++ b/libavcodec/proresenc.c
> @@ -1148,11 +1148,22 @@ static av_cold int encode_init(AVCodecContext *avctx)
>  return AVERROR(EINVAL);
>  }
>  if (av_pix_fmt_desc_get(avctx->pix_fmt)->flags & AV_PIX_FMT_FLAG_ALPHA) {
> -if (ctx->alpha_bits & 7) {
> +if (ctx->profile != PRORES_PROFILE_ &&
> +avctx->strict_std_compliance > FF_COMPLIANCE_NORMAL) {

I'm not good at this stuff - does that compliance level mean this warning will
be printed for normal user feeding yuva10p to the encoder with default
profile?

> +// ignore alpha but warn
> +av_log(avctx, AV_LOG_WARNING, "If you want alpha to be "
> +   "encoded, please specify -profile \n");
> +ctx->alpha_bits = 0;
> +} else if (ctx->alpha_bits & 7) {
>  av_log(avctx, AV_LOG_ERROR, "alpha bits should be 0, 8 or 16\n");
>  return AVERROR(EINVAL);
>  }
>  } else {
> +if (ctx->profile == PRORES_PROFILE_ &&
> +avctx->strict_std_compliance > FF_COMPLIANCE_NORMAL)
> +av_log(avctx, AV_LOG_WARNING, "profile with alpha but "
> +   "content has no alpha\n");
> +

As much as I read
https://documentation.apple.com/en/finalcutpro/professionalformatsandworkflows/index.html#chapter=10%26section=2%26tasks=true
it seems that alpha is not mandatory there.

>  ctx->alpha_bits = 0;
>  }
>  
> -- 
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] proresenc: warn/reject on incorrect profile

2014-08-18 Thread Kostya Shishkov
On Mon, Aug 18, 2014 at 04:22:28PM +0200, Christophe Gisquet wrote:
> Hi,
> 
> following the discussion with Konstantin, prores encoder author,
> here's a proposal matching his preference.
> 
> The alternative of changing the profile to match the colorspace is
> more user-friendly (whatever his level of understanding of what he is
> doing) and has some support, but I see advantages with the other
> option so I don't mind either.
> 
> -- 
> Christophe

> From db85b7a84e498413a71e1d441eea2be6b35ae2e5 Mon Sep 17 00:00:00 2001
> From: Christophe Gisquet 
> Date: Mon, 18 Aug 2014 11:27:50 +0200
> Subject: [PATCH] proresenc: warn/reject on incorrect profile
> 
> This fixes the following situations:
> - Reject encoding when profile selected encodes alpha, but the pixel format
>   does not have an alpha channel;

That looks very wrong - i.e. by default we have alpha_bits=16 and nothing
should stop user from feeding YUV444(no alpha) for  profile. Currently it
simply won't attempt to encode alpha, with your change it will error out.

> - Warn if the pixel format has an alpha channel but the profile does not
>   encode it;
> - Do not encode alpha if the profile does not specify it.

In general I'd reformulate it differently - "allow alpha plane encoding only
with  profile because of original software yada yada blah"

> ---
>  libavcodec/proresenc.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/proresenc.c b/libavcodec/proresenc.c
> index bbb414e..8b41789 100644
> --- a/libavcodec/proresenc.c
> +++ b/libavcodec/proresenc.c
> @@ -1148,11 +1148,22 @@ static av_cold int encode_init(AVCodecContext *avctx)
>  return AVERROR(EINVAL);
>  }
>  if (av_pix_fmt_desc_get(avctx->pix_fmt)->flags & AV_PIX_FMT_FLAG_ALPHA) {
> -if (ctx->alpha_bits & 7) {
> +if (ctx->profile != PRORES_PROFILE_) {

Maybe add strictness checking level so that people can force alpha encoding
with other profiles? Original decoder will ignore it, ours won't.

> +// ignore alpha but warn
> +av_log(avctx, AV_LOG_WARNING, "If you want alpha to be encoded,"
> +   "please specify -profile \n");
> +ctx->alpha_bits = 0;
> +}
> +else if (ctx->alpha_bits & 7) {
>  av_log(avctx, AV_LOG_ERROR, "alpha bits should be 0, 8 or 16\n");
>  return AVERROR(EINVAL);
>  }
>  } else {
> +if (ctx->profile == PRORES_PROFILE_) {
> +av_log(avctx, AV_LOG_ERROR, "attempt to encode alpha while "
> +   "content has no alpha\n");
> +return AVERROR(EINVAL);
> +}
>  ctx->alpha_bits = 0;
>  }
>  
> -- 
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] tiff: return proper error for missing LZMA compression.

2014-08-18 Thread Kostya Shishkov
On Mon, Aug 18, 2014 at 09:27:49AM +0100, Diego Elio Pettenò wrote:
> The LZMA support is a semi-official extension supported by libtiff 4.0.0
> and later.
> 
> Signed-off-by: Diego Elio Pettenò 
> ---
>  libavcodec/tiff.c | 3 +++
>  libavcodec/tiff.h | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> index ca5ec75..3b2fc7d 100644
> --- a/libavcodec/tiff.c
> +++ b/libavcodec/tiff.c
> @@ -408,6 +408,9 @@ static int tiff_decode_tag(TiffContext *s)
>  case TIFF_NEWJPEG:
>  avpriv_report_missing_feature(s->avctx, "JPEG compression");
>  return AVERROR_PATCHWELCOME;
> +case TIFF_LZMA:
> +avpriv_report_missing_feature(s->avctx, "LZMA compression");
> +return AVERROR_PATCHWELCOME;
>  default:
>  av_log(s->avctx, AV_LOG_ERROR, "Unknown compression method %i\n",
> s->compr);
> diff --git a/libavcodec/tiff.h b/libavcodec/tiff.h
> index 8a3f7f7..68ac695 100644
> --- a/libavcodec/tiff.h
> +++ b/libavcodec/tiff.h
> @@ -71,7 +71,8 @@ enum TiffCompr {
>  TIFF_NEWJPEG,
>  TIFF_ADOBE_DEFLATE,
>  TIFF_PACKBITS = 0x8005,
> -TIFF_DEFLATE  = 0x80B2
> +TIFF_DEFLATE  = 0x80B2,
> +TIFF_LZMA = 0x886D,
>  };
>  
>  enum TiffTypes {
> -- 

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


Re: [libav-devel] [PATCH] tiff: return proper error for missing LZMA compression.

2014-08-17 Thread Kostya Shishkov
On Sun, Aug 17, 2014 at 10:53:27PM +0100, Diego Elio Pettenò wrote:
> The LZMA support was added to tiff 4.0.0. Whle not very common, it's nice
  ^ whIle
> to tell the user why the file is not working.

Also what's the versioning? IIRC the last official standard was TIFF 6.0 

> Signed-off-by: Diego Elio Pettenò 
> ---
>  libavcodec/tiff.c | 3 +++
>  libavcodec/tiff.h | 3 ++-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> index ca5ec75..3b2fc7d 100644
> --- a/libavcodec/tiff.c
> +++ b/libavcodec/tiff.c
> @@ -408,6 +408,9 @@ static int tiff_decode_tag(TiffContext *s)
>  case TIFF_NEWJPEG:
>  avpriv_report_missing_feature(s->avctx, "JPEG compression");
>  return AVERROR_PATCHWELCOME;
> +case TIFF_LZMA:
> +avpriv_report_missing_feature(s->avctx, "LZMA compression");
> +return AVERROR_PATCHWELCOME;
>  default:
>  av_log(s->avctx, AV_LOG_ERROR, "Unknown compression method %i\n",
> s->compr);
> diff --git a/libavcodec/tiff.h b/libavcodec/tiff.h
> index 8a3f7f7..c42fa41 100644
> --- a/libavcodec/tiff.h
> +++ b/libavcodec/tiff.h
> @@ -71,7 +71,8 @@ enum TiffCompr {
>  TIFF_NEWJPEG,
>  TIFF_ADOBE_DEFLATE,
>  TIFF_PACKBITS = 0x8005,
> -TIFF_DEFLATE  = 0x80B2
> +TIFF_DEFLATE  = 0x80B2,
> +TIFF_LZMA = 0x886d
>  };
>  
>  enum TiffTypes {
> -- 

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


Re: [libav-devel] [PATCH 4/4] proresenc: properly account for alpha

2014-08-11 Thread Kostya Shishkov
On Mon, Aug 11, 2014 at 10:04:24PM +, Christophe Gisquet wrote:
> The packet buffer allocation considered as dct-coded, while it is
> actually run-coded and thus requires a larger buffer.
> ---
>  libavcodec/proresenc.c | 13 ++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/proresenc.c b/libavcodec/proresenc.c
> index e254a3b..391c762 100644
> --- a/libavcodec/proresenc.c
> +++ b/libavcodec/proresenc.c
> @@ -1234,16 +1234,23 @@ static av_cold int encode_init(AVCodecContext *avctx)
>  ctx->bits_per_mb = ls * 8;
>  if (ctx->chroma_factor == CFACTOR_Y444)
>  ctx->bits_per_mb += ls * 4;
> -if (ctx->num_planes == 4)
> -ctx->bits_per_mb += ls * 4;
>  }
>  
>  ctx->frame_size_upper_bound = ctx->pictures_per_frame *
>ctx->slices_per_picture *
> -  (2 + 2 * ctx->num_planes +
> +  (2 + 2 * FFMIN(3, ctx->num_planes) +

This part was right, it's for 2-byte size entries in slice (total size plus
plane sizes except for the last one), so don't touch it.

> (mps * ctx->bits_per_mb) / 8)
>+ 200;
>  
> +if (ctx->alpha_bits) {
> + // alpha plane is run-coded and might run over bit budget
> + ctx->frame_size_upper_bound += (ctx->pictures_per_frame *
> + ctx->slices_per_picture *
> + ctx->mbs_per_slice *
> + 256 * (1 + ctx->alpha_bits+1)
> + + 7) >> 3;

it should be

 ctx->frame_size_upper_bound +=  ctx->pictures_per_frame *
 ctx->slices_per_picture *
/* num pixels per slice */  (ctx->mbs_per_slice * 256 *
/* bits per pixel */ (1 + ctx->alpha_bits + 1) + 7 >> 3);

I.e. total slices multiplied by the rounded up number of bytes per slice.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] swscale: prevent an invalid write of 1 byte

2014-08-03 Thread Kostya Shishkov
On Sun, Aug 03, 2014 at 10:35:35AM +0100, Vittorio Giovara wrote:
> Bug-Id: 772
> CC: libav-sta...@libav.org
> Found-By: Justin Ruggles 
> ---
>  libswscale/swscale_unscaled.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/libswscale/swscale_unscaled.c b/libswscale/swscale_unscaled.c
> index 2907918..9ef456e 100644
> --- a/libswscale/swscale_unscaled.c
> +++ b/libswscale/swscale_unscaled.c
> @@ -670,10 +670,6 @@ static int rgbToRgbWrapper(SwsContext *c, const uint8_t 
> *src[], int srcStride[],
>  !isRGBA32(dstFormat))
>  srcPtr += ALT32_CORR;
>  
> -if ((dstFormat == AV_PIX_FMT_RGB32_1 || dstFormat == 
> AV_PIX_FMT_BGR32_1) &&
> -!isRGBA32(srcFormat))
> -dstPtr += ALT32_CORR;
> -
>  if (dstStride[0] * srcBpp == srcStride[0] * dstBpp && srcStride[0] > 
> 0 &&
>  !(srcStride[0] % srcBpp))
>  conv(srcPtr, dstPtr + dstStride[0] * srcSliceY,
> -- 

I have a strong suspicion it breaks something else there. But swscale is such
a beautiful code after all!
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] tiff: Replace deprecated PIX_FMT names by modern ones

2014-08-02 Thread Kostya Shishkov
On Sat, Aug 02, 2014 at 08:48:20AM -0700, Diego Biurrun wrote:
> ---
>  libavcodec/tiff.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
> index 6c72dc8..6afc2cb 100644
> --- a/libavcodec/tiff.c
> +++ b/libavcodec/tiff.c
> @@ -635,13 +635,13 @@ static int decode_frame(AVCodecContext *avctx,
>  dst   = p->data[0];
>  soff  = s->bpp >> 3;
>  ssize = s->width * soff;
> -if (s->avctx->pix_fmt == PIX_FMT_RGB48LE) {
> +if (s->avctx->pix_fmt == AV_PIX_FMT_RGB48LE) {
>  for (i = 0; i < s->height; i++) {
>  for (j = soff; j < ssize; j += 2)
>  AV_WL16(dst + j, AV_RL16(dst + j) + AV_RL16(dst + j - 
> soff));
>  dst += stride;
>  }
> -} else if (s->avctx->pix_fmt == PIX_FMT_RGB48BE) {
> +} else if (s->avctx->pix_fmt == AV_PIX_FMT_RGB48BE) {
>  for (i = 0; i < s->height; i++) {
>  for (j = soff; j < ssize; j += 2)
>  AV_WB16(dst + j, AV_RB16(dst + j) + AV_RB16(dst + j - 
> soff));
> -- 

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


Re: [libav-devel] [PATCH] png: disable broken MMX/SIMD code for bpp > 2

2014-08-01 Thread Kostya Shishkov
On Fri, Aug 01, 2014 at 08:33:11PM +0200, Luca Barbato wrote:
> On 01/08/14 20:19, Vittorio Giovara wrote:
> > The decoder was producing different results when ASM was disabled.
> > Based on a long debug session with Kostya.
> > ---
> >  libavcodec/pngdec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > index 826edab..fa7f7cc 100644
> > --- a/libavcodec/pngdec.c
> > +++ b/libavcodec/pngdec.c
> > @@ -240,7 +240,7 @@ static void png_filter_row(PNGDSPContext *dsp, uint8_t 
> > *dst, int filter_type,
> >  p  = last[i];
> >  dst[i] = p + src[i];
> >  }
> > -if (bpp > 1 && size > 4) {
> > +if (bpp > 2 && size > 4) {
> >  /* would write off the end of the array if we let it process
> >   * the last pixel with bpp=3 */
> >  int w = bpp == 4 ? size : size - 3;
> > 
> 
> Ok.

Except for the message, which is wrong.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/3] png: fix macro expansion/parenthesis

2014-07-31 Thread Kostya Shishkov
On Thu, Jul 31, 2014 at 04:01:50PM +0200, Luca Barbato wrote:
> On 31/07/14 15:38, Vittorio Giovara wrote:
> > Partially based on a patch by Michael Niedermayer .
> > ---
> >  libavcodec/pngdec.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> 
> Is there more of it around? Does it cause any side effect warranting
> backporting?

The main question should be: does it really _fix_ anything or is it there just
to make LISP developers feel better?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] h264: prevent theoretical infinite loop in SEI parsing

2014-07-30 Thread Kostya Shishkov
On Wed, Jul 30, 2014 at 07:52:01PM +0100, Vittorio Giovara wrote:
> Properly address CVE-2011-3946 and parse bitstream as described in the spec.
> 
> CC: libav-sta...@libav.org
> Found-by: Mateusz "j00ru" Jurczyk and Gynvael Coldwind
> ---
>  libavcodec/h264_sei.c | 17 +++--
>  1 file changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c
> index 33230b7..641ee1d 100644
> --- a/libavcodec/h264_sei.c
> +++ b/libavcodec/h264_sei.c
> @@ -222,14 +222,19 @@ int ff_h264_decode_sei(H264Context *h)
>  int size = 0;
>  int type = 0;
>  int ret  = 0;
> +int last = 0;
>  
> -do
> -type += show_bits(&h->gb, 8);
> -while (get_bits(&h->gb, 8) == 255);
> +while (get_bits_left(&h->gb) >= 8 &&
> +   (last = get_bits(&h->gb, 8)) == 255) {
> +type += 255;
> +}
> +type += last;
>  
> -do
> -size += show_bits(&h->gb, 8);
> -while (get_bits(&h->gb, 8) == 255);

last = 0 missing here?

> +while (get_bits_left(&h->gb) >= 8 &&
> +   (last = get_bits(&h->gb, 8)) == 255) {
> +size += 255;
> +}
> +size += last;
>  
>  if (size > get_bits_left(&h->gb) / 8) {
>  av_log(h->avctx, AV_LOG_ERROR, "SEI type %d truncated at %d\n",
> -- 
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/6] vc1dsp: Add wrappers for {avg|put}_vc1_mspel_mc00_c

2014-07-24 Thread Kostya Shishkov
On Thu, Jul 24, 2014 at 05:03:26PM -0700, Diego Biurrun wrote:
> This avoids invoking the wrapped functions with too many arguments.
> ---
>  libavcodec/vc1dsp.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

looks OK for a change
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 3/6] svq1: Drop const from a pointer variable to eliminate a compiler warning

2014-07-24 Thread Kostya Shishkov
On Thu, Jul 24, 2014 at 05:03:27PM -0700, Diego Biurrun wrote:
> ---
>  libavcodec/svq1dec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/svq1dec.c b/libavcodec/svq1dec.c
> index 000487b..d13fc02 100644
> --- a/libavcodec/svq1dec.c
> +++ b/libavcodec/svq1dec.c
> @@ -607,7 +607,7 @@ static int svq1_decode_frame_header(AVCodecContext 
> *avctx, AVFrame *frame)
>  static int svq1_decode_frame(AVCodecContext *avctx, void *data,
>   int *got_frame, AVPacket *avpkt)
>  {
> -const uint8_t *buf = avpkt->data;
> +uint8_t *buf   = avpkt->data;
>  int buf_size   = avpkt->size;
>  SVQ1Context *s = avctx->priv_data;
>  AVFrame   *cur = data;
> -- 

It's there for a reason too - because you have a WTFy block below that
actually modifies input data.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 4/6] lcl: Eliminate silly pointer variable indirection

2014-07-24 Thread Kostya Shishkov
On Thu, Jul 24, 2014 at 05:03:28PM -0700, Diego Biurrun wrote:
> libavcodec/lcldec.c:165:30: warning: cast discards ‘__attribute__((const))’ 
> qualifier from pointer target type
> ---
>  libavcodec/lcldec.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/libavcodec/lcldec.c b/libavcodec/lcldec.c
> index 4d97948..f2af815 100644
> --- a/libavcodec/lcldec.c
> +++ b/libavcodec/lcldec.c
> @@ -159,10 +159,9 @@ static int zlib_decomp(AVCodecContext *avctx, const 
> uint8_t *src, int src_len, i
>  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame, 
> AVPacket *avpkt)
>  {
>  AVFrame *frame = data;
> -const uint8_t *buf = avpkt->data;
>  int buf_size = avpkt->size;
>  LclDecContext * const c = avctx->priv_data;
> -unsigned char *encoded = (unsigned char *)buf;
> +unsigned char *encoded = avpkt->data;
>  unsigned int pixel_ptr;
>  int row, col;
>  unsigned char *outptr;
> -- 

Here it's even funnier because the same variable is reused in two cases
(otherwise it could've been made const too).
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 6/6] Drop const qualifier from pointers passed to zlib functions

2014-07-24 Thread Kostya Shishkov
On Thu, Jul 24, 2014 at 05:03:30PM -0700, Diego Biurrun wrote:
> This matches the zlib API and avoids compiler warnings about const qualifiers.
> ---
>  libavcodec/lcldec.c | 2 +-
>  libavcodec/tiff.c   | 6 +++---
>  libavcodec/zmbv.c   | 2 +-
>  3 files changed, 5 insertions(+), 5 deletions(-)

This does not feel right, it's zlib API that should be fixed.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 5/6] tscc: Eliminate silly pointer variable indirection

2014-07-24 Thread Kostya Shishkov
On Thu, Jul 24, 2014 at 05:03:29PM -0700, Diego Biurrun wrote:
> libavcodec/tscc.c:85:24: warning: assignment discards ‘const’ qualifier from 
> pointer target type
> ---
>  libavcodec/tscc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/libavcodec/tscc.c b/libavcodec/tscc.c
> index c26853e..fdc5cd6 100644
> --- a/libavcodec/tscc.c
> +++ b/libavcodec/tscc.c
> @@ -64,10 +64,9 @@ typedef struct TsccContext {
>  static int decode_frame(AVCodecContext *avctx, void *data, int *got_frame,
>  AVPacket *avpkt)
>  {
> -const uint8_t *buf = avpkt->data;
>  int buf_size = avpkt->size;
>  CamtasiaContext * const c = avctx->priv_data;
> -const unsigned char *encoded = buf;
> +unsigned char *encoded = avpkt->data;
>  AVFrame *frame = data;
>  int zret; // Zlib return code
>  int ret, len = buf_size;
> -- 

It's the same zlib shit, the same reply
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/5] avutil: rename AV_PIX_FMT_Y400A to AV_PIX_FMT_GRAY8A

2014-07-23 Thread Kostya Shishkov
On Wed, Jul 23, 2014 at 10:05:28AM +0100, Vittorio Giovara wrote:
> On Wed, Jul 23, 2014 at 8:52 AM, Kostya Shishkov
>  wrote:
> > On Wed, Jul 23, 2014 at 08:47:59AM +0100, Vittorio Giovara wrote:
> >> On Mon, Jul 21, 2014 at 8:43 PM, Anton Khirnov  wrote:
> >> >
> >> > On Sun, 20 Jul 2014 23:59:17 +0100, Vittorio Giovara 
> >> >  wrote:
> >> >> @@ -256,6 +256,7 @@ enum AVPixelFormat {
> >> >>  #define AV_PIX_FMT_XYZ12  AV_PIX_FMT_NE(XYZ12BE, XYZ12LE)
> >> >>  #define AV_PIX_FMT_NV20   AV_PIX_FMT_NE(NV20BE,  NV20LE)
> >> >>
> >> >> +#define AV_PIX_FMT_Y400A  AV_PIX_FMT_GRAY8A
> >> >>
> >> >
> >> > Why make it a macro instead of an additional enum value?
> >> >
> >> > Also, missing APIchanges entry.
> >>
> >> if i do (before AV_PIX_FMT_NB)
> >> +AV_PIX_FMT_Y400A  =AV_PIX_FMT_GRAY8A,
> >> and
> >> -#define AV_PIX_FMT_Y400A  AV_PIX_FMT_GRAY8A
> >>
> >> i get several errors in swscale when compiling with clang
> >>
> >> libswscale/utils.c:95:6: error: array designator index (119) exceeds array
> >>   bounds (67)
> >> [AV_PIX_FMT_YVYU422] = { 1, 1 },
> >>  ^~
> >>
> >> Shoudl I put it after AV_PIX_FMT_NB? (if I do the error disappears)
> >
> > That happens because if you don't specify value it would be previous enum
> > value plus one (see 6.7.2.2p3). So the only good place to add it is right
> > immediately after AV_PIX_FMT_GRAY8A.
> >
> 
> Ok I see now, thanks.
> 
> > (And this rename still looks uncalled for)
> 
> Would the group prefer renaming Y400A to Y8A and the new format to
> Y16A, as suggested on IRC?

In case rationale is not clear: you have packed format in form
   
and shortening greyscale to 'G' might make one thing about Greenscale instead.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/5] avutil: rename AV_PIX_FMT_Y400A to AV_PIX_FMT_GRAY8A

2014-07-23 Thread Kostya Shishkov
On Wed, Jul 23, 2014 at 08:47:59AM +0100, Vittorio Giovara wrote:
> On Mon, Jul 21, 2014 at 8:43 PM, Anton Khirnov  wrote:
> >
> > On Sun, 20 Jul 2014 23:59:17 +0100, Vittorio Giovara 
> >  wrote:
> >> @@ -256,6 +256,7 @@ enum AVPixelFormat {
> >>  #define AV_PIX_FMT_XYZ12  AV_PIX_FMT_NE(XYZ12BE, XYZ12LE)
> >>  #define AV_PIX_FMT_NV20   AV_PIX_FMT_NE(NV20BE,  NV20LE)
> >>
> >> +#define AV_PIX_FMT_Y400A  AV_PIX_FMT_GRAY8A
> >>
> >
> > Why make it a macro instead of an additional enum value?
> >
> > Also, missing APIchanges entry.
> 
> if i do (before AV_PIX_FMT_NB)
> +AV_PIX_FMT_Y400A  =AV_PIX_FMT_GRAY8A,
> and
> -#define AV_PIX_FMT_Y400A  AV_PIX_FMT_GRAY8A
> 
> i get several errors in swscale when compiling with clang
> 
> libswscale/utils.c:95:6: error: array designator index (119) exceeds array
>   bounds (67)
> [AV_PIX_FMT_YVYU422] = { 1, 1 },
>  ^~
> 
> Shoudl I put it after AV_PIX_FMT_NB? (if I do the error disappears)

That happens because if you don't specify value it would be previous enum
value plus one (see 6.7.2.2p3). So the only good place to add it is right
immediately after AV_PIX_FMT_GRAY8A.

(And this rename still looks uncalled for)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/2] Added two tests for the old RealAudio formats, and their refs.

2014-07-16 Thread Kostya Shishkov
On Wed, Jul 16, 2014 at 03:27:26PM +0200, Diego Biurrun wrote:
> On Sun, Jul 13, 2014 at 11:47:06PM +0200, Katerina Barone-Adesi wrote:
> > --- a/tests/fate/real.mak
> > +++ b/tests/fate/real.mak
> > @@ -1,3 +1,11 @@
> > +# NOTE: the REALAUDIO tests need to use the RA demuxer once
> > +# it is available
> 
> Such a thing is going to be available when?
> 
> > +FATE_SAMPLES_REALAUDIO-$(call DEMDEC, RM, RA_144) += fate-ra3-144
> > +fate-ra3-144: CMD = framecrc -i $(TARGET_SAMPLES)/realaudio/ra3.ra
> > +
> > +FATE_SAMPLES_REALAUDIO-$(call DEMDEC, RM, RA_288) += fate-ra4-28_8
> > +fate-ra4-28_8: CMD = framecrc -i $(TARGET_SAMPLES)/realaudio/ra4-28_8.ra
> 
> The inconsistent naming is puzzling.  One sample is just ra3.ra, the other
> is ra-28_8.ra.  Also note the inconsistent use of _.

Maybe because it's not possible to have RA3 with any other codec than 14.4?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 4/4] dcadec: Support for xll (lossless extension)

2014-07-05 Thread Kostya Shishkov
On Sat, Jul 05, 2014 at 03:55:12PM +0100, Derek Buitenhuis wrote:
> On 6/26/2014 9:09 AM, Niels Möller wrote:
> > Ping?
> 
> I guess the only one who is qualified to technically review DTS stuff is 
> Kostya...

Pity I'm legally challenged not to.
 
> Apologies for the patch rot, I'm not sure what should be done
> before merge besides patch fix-ups noted above (can be done by
> whoever, before merge), and the various FIXMEs + channel layout
> stuff.
> 
> Maybe someone else can comment who has an opinion on what needs
> to be done next, rather than let the set languish here...

I'd say - bitexact core reconstruction and less conversion from/to floats but
that's just me.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] matroskadec: Fix a double negation typo

2014-07-02 Thread Kostya Shishkov
On Wed, Jul 02, 2014 at 02:38:53PM +0300, Martin Storsjö wrote:
> From: Michael Niedermayer 
> 
> This typo has existed since this code was added in c16582579.
> Newer versions of clang pointed out that this comparison always
> was true (since the result of the negation is either 0 or 1, while
> AVDISCARD_ALL has the value 48).
> ---
>  libavformat/matroskadec.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index d5bc923..22108ad 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -2560,7 +2560,7 @@ static int matroska_read_seek(AVFormatContext *s, int 
> stream_index,
>  tracks[i].audio.buf_timecode   = AV_NOPTS_VALUE;
>  tracks[i].end_timecode = 0;
>  if (tracks[i].type == MATROSKA_TRACK_TYPE_SUBTITLE &&
> -!tracks[i].stream->discard != AVDISCARD_ALL) {
> +tracks[i].stream->discard != AVDISCARD_ALL) {
>  index_sub = av_index_search_timestamp(
>  tracks[i].stream, st->index_entries[index].timestamp,
>  AVSEEK_FLAG_BACKWARD);
> -- 

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


Re: [libav-devel] [PATCH] vaapi: Update idct_permutation location after dsputil/idctdsp split

2014-07-01 Thread Kostya Shishkov
On Tue, Jul 01, 2014 at 07:30:48AM -0700, Diego Biurrun wrote:
> ---
> 
> Yeah, yeah, brown paper bag ...
> 
>  libavcodec/vaapi_mpeg2.c | 2 +-
>  libavcodec/vaapi_mpeg4.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/vaapi_mpeg2.c b/libavcodec/vaapi_mpeg2.c
> index 03fb670..31e0218 100644
> --- a/libavcodec/vaapi_mpeg2.c
> +++ b/libavcodec/vaapi_mpeg2.c
> @@ -90,7 +90,7 @@ static int vaapi_mpeg2_start_frame(AVCodecContext *avctx, 
> av_unused const uint8_
>  iq_matrix->load_chroma_non_intra_quantiser_matrix   = 1;
>  
>  for (i = 0; i < 64; i++) {
> -int n = s->dsp.idct_permutation[ff_zigzag_direct[i]];
> +int n = s->idsp.idct_permutation[ff_zigzag_direct[i]];
>  iq_matrix->intra_quantiser_matrix[i]= s->intra_matrix[n];
>  iq_matrix->non_intra_quantiser_matrix[i]= s->inter_matrix[n];
>  iq_matrix->chroma_intra_quantiser_matrix[i] = 
> s->chroma_intra_matrix[n];
> diff --git a/libavcodec/vaapi_mpeg4.c b/libavcodec/vaapi_mpeg4.c
> index fc3a15e..abdb6d9 100644
> --- a/libavcodec/vaapi_mpeg4.c
> +++ b/libavcodec/vaapi_mpeg4.c
> @@ -109,7 +109,7 @@ static int vaapi_mpeg4_start_frame(AVCodecContext *avctx, 
> av_unused const uint8_
>  iq_matrix->load_non_intra_quant_mat = 1;
>  
>  for (i = 0; i < 64; i++) {
> -int n = s->dsp.idct_permutation[ff_zigzag_direct[i]];
> +int n = s->idsp.idct_permutation[ff_zigzag_direct[i]];
>  iq_matrix->intra_quant_mat[i]   = s->intra_matrix[n];
>  iq_matrix->non_intra_quant_mat[i]   = s->inter_matrix[n];
>  }
> -- 

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


Re: [libav-devel] [PATCH] mjpeg: Split off bits shared by MJPEG and LJPEG encoders

2014-06-30 Thread Kostya Shishkov
On Mon, Jun 30, 2014 at 07:58:42AM -0700, Diego Biurrun wrote:
> This obviates a dependency of the LJPEG encoder on mpegvideo.
> ---
> 
> mjpegenc_common it is now; in the mjpeg namespace.
> 
>  configure|   2 +-
>  libavcodec/Makefile  |   4 +-
>  libavcodec/ljpegenc.c|   1 +
>  libavcodec/mjpegenc.c| 286 
> +--
>  libavcodec/mjpegenc.h|   7 -
>  libavcodec/{mjpegenc.c => mjpegenc_common.c} | 151 +-
>  libavcodec/{mjpegenc.h => mjpegenc_common.h} |  39 +---
>  libavcodec/mpegvideo_enc.c   |   1 +
>  8 files changed, 21 insertions(+), 470 deletions(-)
>  copy libavcodec/{mjpegenc.c => mjpegenc_common.c} (67%)
>  copy libavcodec/{mjpegenc.h => mjpegenc_common.h} (52%)

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


Re: [libav-devel] [PATCH] mjpeg: Split off bits shared by MJPEG and LJPEG encoders

2014-06-30 Thread Kostya Shishkov
On Mon, Jun 30, 2014 at 04:30:18PM +0200, Diego Biurrun wrote:
> On Mon, Jun 30, 2014 at 04:20:02PM +0200, Kostya Shishkov wrote:
> > On Mon, Jun 30, 2014 at 07:15:47AM -0700, Diego Biurrun wrote:
> > > This obviates a dependency of the LJPEG encoder on mpegvideo.
> > > ---
> > > 
> > > Renamed everything to ljpeg, as suggested by Kostya.
> > > 
> > >  configure  |   2 +-
> > >  libavcodec/Makefile|   4 +-
> > >  libavcodec/{mjpegenc.c => ljpeg.c} | 161 ++--
> > >  libavcodec/ljpeg.h |  38 +
> > >  libavcodec/ljpegenc.c  |  27 +++-
> > >  libavcodec/mjpegenc.c  | 292 
> > > +
> > >  libavcodec/mjpegenc.h  |   7 -
> > >  libavcodec/mpegvideo_enc.c |   8 +-
> > >  8 files changed, 83 insertions(+), 456 deletions(-)
> > >  copy libavcodec/{mjpegenc.c => ljpeg.c} (65%)
> > >  create mode 100644 libavcodec/ljpeg.h
> > 
> > Is it just me or you rename some encoding functions from ff_mjpeg_* to
> > ff_ljpeg_* and yet still use them for both (M)JPEG and LJPEG encoding?
> 
> Yes, I do.  I expect functions exported from ljpeg.c to live in the ljpeg
> namespace.  Did I misunderstand your original suggestion?  What is the
> alternative?  Keep the mjpeg namespace?

If it's used in both then it is pure JPEG functions that should not be renamed
to Ljpeg namespace (and mjpegenc_common.[ch] would be more appropriate for
them).
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] mjpeg: Split off bits shared by MJPEG and LJPEG encoders

2014-06-30 Thread Kostya Shishkov
On Mon, Jun 30, 2014 at 07:15:47AM -0700, Diego Biurrun wrote:
> This obviates a dependency of the LJPEG encoder on mpegvideo.
> ---
> 
> Renamed everything to ljpeg, as suggested by Kostya.
> 
>  configure  |   2 +-
>  libavcodec/Makefile|   4 +-
>  libavcodec/{mjpegenc.c => ljpeg.c} | 161 ++--
>  libavcodec/ljpeg.h |  38 +
>  libavcodec/ljpegenc.c  |  27 +++-
>  libavcodec/mjpegenc.c  | 292 
> +
>  libavcodec/mjpegenc.h  |   7 -
>  libavcodec/mpegvideo_enc.c |   8 +-
>  8 files changed, 83 insertions(+), 456 deletions(-)
>  copy libavcodec/{mjpegenc.c => ljpeg.c} (65%)
>  create mode 100644 libavcodec/ljpeg.h

Is it just me or you rename some encoding functions from ff_mjpeg_* to
ff_ljpeg_* and yet still use them for both (M)JPEG and LJPEG encoding?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] mjpeg: Split off bits shared by MJPEG and LJPEG encoders

2014-06-30 Thread Kostya Shishkov
On Mon, Jun 30, 2014 at 05:56:54AM -0700, Diego Biurrun wrote:
> This obviates a dependency of the LJPEG encoder on mpegvideo.
> ---
> 
> A suggestion for a better name than lmjpeg is welcome ..

It's ljpeg and should remain so.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] indeo2: rename stride to pitch for consistency with other Indeo decoders

2014-06-26 Thread Kostya Shishkov
On Thu, Jun 26, 2014 at 12:15:36AM +0200, Diego Biurrun wrote:
> On Wed, Jun 25, 2014 at 08:29:29PM +0200, Kostya Shishkov wrote:
> > ---
> >  libavcodec/indeo2.c |   16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> I strongly vote for my patch instead to have things more, not less,
> consistent across the codebase.  You trolled me into whipping it up,
> now accept it ;-p

No, your patch is to have things less, not more.
I prefer to have some variety inside codebase, otherwise it will end like
Soviet army where they painted grass green in order to conform with
regulations.

Of course having complete chaos is not good but not being able to name
variables in the way you prefer would be too sad.

P.S. Your patch sucks because it should rename both stride and pitch to
linesize for real consistency.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


[libav-devel] [PATCH] indeo2: rename stride to pitch for consistency with other Indeo decoders

2014-06-25 Thread Kostya Shishkov
---
 libavcodec/indeo2.c |   16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/libavcodec/indeo2.c b/libavcodec/indeo2.c
index 7df6e69..4221e9e 100644
--- a/libavcodec/indeo2.c
+++ b/libavcodec/indeo2.c
@@ -49,7 +49,7 @@ static inline int ir2_get_code(GetBitContext *gb)
 }
 
 static int ir2_decode_plane(Ir2Context *ctx, int width, int height, uint8_t 
*dst,
-int stride, const uint8_t *table)
+int pitch, const uint8_t *table)
 {
 int i;
 int j;
@@ -74,7 +74,7 @@ static int ir2_decode_plane(Ir2Context *ctx, int width, int 
height, uint8_t *dst
 dst[out++] = table[(c * 2) + 1];
 }
 }
-dst += stride;
+dst += pitch;
 
 for (j = 1; j < height; j++) {
 out = 0;
@@ -85,27 +85,27 @@ static int ir2_decode_plane(Ir2Context *ctx, int width, int 
height, uint8_t *dst
 if (out + c*2 > width)
 return AVERROR_INVALIDDATA;
 for (i = 0; i < c * 2; i++) {
-dst[out] = dst[out - stride];
+dst[out] = dst[out - pitch];
 out++;
 }
 } else { /* add two deltas from table */
-t= dst[out - stride] + (table[c * 2] - 128);
+t= dst[out - pitch] + (table[c * 2] - 128);
 t= av_clip_uint8(t);
 dst[out] = t;
 out++;
-t= dst[out - stride] + (table[(c * 2) + 1] - 128);
+t= dst[out - pitch] + (table[(c * 2) + 1] - 128);
 t= av_clip_uint8(t);
 dst[out] = t;
 out++;
 }
 }
-dst += stride;
+dst += pitch;
 }
 return 0;
 }
 
 static int ir2_decode_plane_inter(Ir2Context *ctx, int width, int height, 
uint8_t *dst,
-  int stride, const uint8_t *table)
+  int pitch, const uint8_t *table)
 {
 int j;
 int out = 0;
@@ -133,7 +133,7 @@ static int ir2_decode_plane_inter(Ir2Context *ctx, int 
width, int height, uint8_
 out++;
 }
 }
-dst += stride;
+dst += pitch;
 }
 return 0;
 }
-- 
1.7.9.5

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


Re: [libav-devel] [PATCH] cosmetics: Replace "pitch" by "stride" when referring to array dimensions

2014-06-25 Thread Kostya Shishkov
On Wed, Jun 25, 2014 at 07:42:51AM -0700, Diego Biurrun wrote:
> The latter is more common throughout the codebase and cannot be confused
> with audio-related pitch.
> ---
>  libavcodec/dvdsubdec.c  |   4 +-
>  libavcodec/indeo3.c |  68 +++
>  libavcodec/indeo4.c |   4 +-
>  libavcodec/indeo5.c |   4 +-
>  libavcodec/ivi_common.c |  58 ++---
>  libavcodec/ivi_common.h |   8 +-
>  libavcodec/ivi_dsp.c| 218 
> ++--
>  libavcodec/ivi_dsp.h|  97 +++--
>  libavcodec/sanm.c   | 106 +++
>  libavcodec/svq1dec.c|  50 +--
>  10 files changed, 324 insertions(+), 293 deletions(-)

This looks too silly to me and it cannot cause confusion IMO.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] Indeo4 B-frames decoding support

2014-06-25 Thread Kostya Shishkov
On Wed, Jun 25, 2014 at 03:24:04PM +0200, Diego Biurrun wrote:
> On Wed, Jun 25, 2014 at 10:01:14AM +0200, Dirk Ausserhaus wrote:
> > From 019230859732a5190838b81c7156378acad8b160 Mon Sep 17 00:00:00 2001
> > From: Dirk Ausserhaus 
> > Date: Sun, 8 Jun 2014 13:44:17 +0200
> > Subject: [PATCH] indeo4: B-frames decoding
> > 
> > --- a/libavcodec/ivi_common.c
> > +++ b/libavcodec/ivi_common.c
> > @@ -340,6 +365,11 @@ av_cold int ff_ivi_init_planes(IVIPlaneDesc *planes, 
> > const IVIPicConfig *cfg)
> >  if (!band->bufs[2])
> >  return AVERROR(ENOMEM);
> >  }
> > +if (is_indeo4) {
> > +band->bufs[3]  = av_mallocz(buf_size);
> > +if (!band->bufs[3])
> > +return AVERROR(ENOMEM);
> > +}
> 
> You leak memory on error here.

It's the same as above, the whole memory will be freed afterwards.
 
> > --- a/libavcodec/ivi_dsp.c
> > +++ b/libavcodec/ivi_dsp.c
> > @@ -762,39 +762,66 @@ void ff_ivi_put_dc_pixel_8x8(const int32_t *in, 
> > int16_t *out, uint32_t pitch,
> > +static void ivi_mc_ ## size ##x## size ## suffix (int16_t *buf, \
> > +\
> > +void ff_ivi_mc_ ## size ##x## size ## suffix (int16_t *buf, const int16_t 
> > *ref_buf, \
> > +
> > +void ff_ivi_mc_avg_ ## size ##x## size ## suffix (int16_t *buf, \
> 
> Drop the space between function name and (.

I can do that before committing.

> > --- a/libavcodec/ivi_dsp.h
> > +++ b/libavcodec/ivi_dsp.h
> > @@ -291,4 +291,52 @@ void ff_ivi_mc_8x8_no_delta(int16_t *buf, const 
> > int16_t *ref_buf, uint32_t pitch
> > +/**
> > + *  8x8 block motion compensation with adding delta
> > + *
> > + *  @param[in,out]  buf  pointer to the block in the current frame 
> > buffer containing delta
> > + *  @param[in]  ref_buf  pointer to the corresponding block in the 
> > backward reference frame
> > + *  @param[in]  ref_buf2 pointer to the corresponding block in the 
> > forward reference frame
> > + *  @param[in]  pitchpitch for moving to the next y line
> > + *  @param[in]  mc_type  interpolation type for backward reference
> > + *  @param[in]  mc_type2 interpolation type for forward reference
> > + */
> > +void ff_ivi_mc_avg_8x8_delta(int16_t *buf, const int16_t *ref_buf, const 
> > int16_t *ref_buf2, uint32_t pitch, int mc_type, int mc_type2);
> 
> This is a long line and I would suggest "stride" instead of "pitch".

It's pitch everywhere except Indeo2 decoder.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] x86: h264dsp: Fix link failure with optimizations disabled

2014-06-25 Thread Kostya Shishkov
On Wed, Jun 25, 2014 at 04:40:56AM -0700, Diego Biurrun wrote:
> With optimzations disabled compilers have trouble doing dead code
> elimination on 'if (foo && 0)' expressions, while 'if (0 && foo)'
> still works, so use the latter to avoid problems.
> 
> Bug-Id: 707
> ---
>  libavcodec/x86/h264dsp_init.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/x86/h264dsp_init.c b/libavcodec/x86/h264dsp_init.c
> index 427662f..134d594 100644
> --- a/libavcodec/x86/h264dsp_init.c
> +++ b/libavcodec/x86/h264dsp_init.c
> @@ -212,7 +212,7 @@ av_cold void ff_h264dsp_init_x86(H264DSPContext *c, const 
> int bit_depth,
>  {
>  int cpu_flags = av_get_cpu_flags();
>  
> -if (chroma_format_idc <= 1 && EXTERNAL_MMXEXT(cpu_flags))
> +if (EXTERNAL_MMXEXT(cpu_flags) && chroma_format_idc <= 1)
>  c->h264_loop_filter_strength = ff_h264_loop_filter_strength_mmxext;
>  
>  if (bit_depth == 8) {
> -- 

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


Re: [libav-devel] Indeo4 B-frames decoding support

2014-06-25 Thread Kostya Shishkov
On Wed, Jun 25, 2014 at 10:01:14AM +0200, Dirk Ausserhaus wrote:
> On Sat, Jun 21, 2014 at 8:41 AM, Kostya Shishkov
>  wrote:
> > On Fri, Jun 20, 2014 at 08:46:11PM +0200, Dirk Ausserhaus wrote:
> >> Here is my patch for decoding B-frames in Indeo4 at last. And another
> >> patch to make things a bit simpler first.
> >
> > At last!
> >
> >> I suppose I have to give some explanation what I did here and why.
> >>
> >> First of all, every band has three buffers—current, reference and one
> >> for scalability. I had to add the fourth buffer for the second
> >> reference and change Indeo4 switch_buffers function to handle the
> >> second reference frame.
> >>
> >> Second, I had to add the second motion vector to the block
> >> information. I chose a new name for it in order to have less impact on
> >> existing code, since making mv_x and mv_y arrays would lead to more
> >> changes in all places and more possible errors.
> >>
> >> Third, I had to extend ivi_mc function to handle B-frames and add some
> >> new functions for bidirectional motion compensation. Motion types are
> >> distinguished by mc_type and mc_type2 variables. Normally they should
> >> contain fractional offset for the motion compensation but I overloaded
> >> it so -1 means no motion vector at all.
> >
> > That sounds reasonable to me.
> >
> >> I've tested it on various Indeo4 and Indeo5 samples and it doesn't
> >> seem to break anything.
> >
> > That's the best part ;)
> 
> Indeed, even better would be if it plays everything.
> 
> 
> >> @@ -331,7 +355,8 @@ av_cold int ff_ivi_init_planes(IVIPlaneDesc *planes, 
> >> const IVIPicConfig *cfg)
> >>  band->aheight  = height_aligned;
> >>  band->bufs[0]  = av_mallocz(buf_size);
> >>  band->bufs[1]  = av_mallocz(buf_size);
> >> -if (!band->bufs[0] || !band->bufs[1])
> >> +band->bufs[3]  = av_mallocz(buf_size);
> >> +if (!band->bufs[0] || !band->bufs[1] || !band->bufs[3])
> >>  return AVERROR(ENOMEM);
> >>
> >>  /* allocate the 3rd band buffer for scalability mode */
> >
> > Is it possible to allocate that buffer only for Indeo4?
> 
> Yes, an additional flag was needed though.
> 
> 
> >> @@ -849,8 +912,14 @@ static int decode_band(IVI45DecContext *ctx,
> >>  av_log(avctx, AV_LOG_ERROR, "Band buffer points to no data!\n");
> >>  return AVERROR_INVALIDDATA;
> >>  }
> >> -band->ref_buf = band->bufs[ctx->ref_buf];
> >> -band->data_ptr = ctx->frame_data + (get_bits_count(&ctx->gb) >> 3);
> >> +if (ctx->is_indeo4 && ctx->frame_type == IVI4_FRAMETYPE_BIDIR) {
> >> +band->ref_buf   = band->bufs[ctx->b_ref_buf];
> >> +band->b_ref_buf = band->bufs[ctx->ref_buf];
> >> +} else {
> >> +band->ref_buf   = band->bufs[ctx->ref_buf];
> >> +band->b_ref_buf = band->bufs[ctx->b_ref_buf];
> >
> > Why set second reference for the frames that are not bidirectional?
> > Maybe set it to NULL instead?
> 
> That's reasonable, I've changed it that way.

> From 019230859732a5190838b81c7156378acad8b160 Mon Sep 17 00:00:00 2001
> From: Dirk Ausserhaus 
> Date: Sun, 8 Jun 2014 13:44:17 +0200
> Subject: [PATCH] indeo4: B-frames decoding
> 
> ---
>  libavcodec/indeo4.c |   49 -
>  libavcodec/indeo5.c |4 +-
>  libavcodec/ivi_common.c |  138 
> ---
>  libavcodec/ivi_common.h |   14 --
>  libavcodec/ivi_dsp.c|   43 +--
>  libavcodec/ivi_dsp.h|   48 
>  6 files changed, 238 insertions(+), 58 deletions(-)

looks good to me
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH 1/1] h264: avoid using uninitialized memory in NEON chroma mc

2014-06-23 Thread Kostya Shishkov
On Thu, Jun 19, 2014 at 12:26:15PM +0200, Janne Grunau wrote:
> Adapt commit 982b596ea6640bfe218a31f6c3fc542d9fe61c31 for the arm and
> aarch64 NEON asm. 5-10% faster on Cortex-A9.
> ---
>  libavcodec/aarch64/h264cmc_neon.S | 59 +++---
>  libavcodec/arm/h264cmc_neon.S | 60 
> ---
>  2 files changed, 111 insertions(+), 8 deletions(-)

on a cursory glance looks OK
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] lzo: Handle integer overflow

2014-06-22 Thread Kostya Shishkov
On Fri, Jun 20, 2014 at 01:14:21PM +0200, Luca Barbato wrote:
> get_len can overflow for specially crafted payload.

when you have lots of zeroes it seems ;)
 
> Reported-By: Don A. Baley 
> CC: libav-sta...@libav.org
> ---
>  libavutil/lzo.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/libavutil/lzo.c b/libavutil/lzo.c
> index 5c5ebc8..e458165 100644
> --- a/libavutil/lzo.c
> +++ b/libavutil/lzo.c
> @@ -80,6 +80,10 @@ static inline void copy(LZOContext *c, int cnt)
>  {
>  register const uint8_t *src = c->in;
>  register uint8_t *dst   = c->out;
> +if (cnt < 0) {
> +c->error |= AV_LZO_ERROR;
> +return;
> +}
>  if (cnt > c->in_end - src) {
>  cnt   = FFMAX(c->in_end - src, 0);
>  c->error |= AV_LZO_INPUT_DEPLETED;
> @@ -103,7 +107,7 @@ static inline void copy(LZOContext *c, int cnt)
>  /**
>   * @brief Copies previously decoded bytes to current position.
>   * @param back how many bytes back we start
> - * @param cnt number of bytes to copy, must be >= 0
> + * @param cnt number of bytes to copy, must be > 0
>   *
>   * cnt > back is valid, this will copy the bytes we just copied,
>   * thus creating a repeating pattern with a period length of back.
> @@ -111,6 +115,10 @@ static inline void copy(LZOContext *c, int cnt)
>  static inline void copy_backptr(LZOContext *c, int back, int cnt)
>  {
>  register uint8_t *dst   = c->out;
> +if (cnt <= 0) {
> +c->error |= AV_LZO_ERROR;
> +return;
> +}
>  if (dst - c->out_start < back) {
>  c->error |= AV_LZO_INVALID_BACKPTR;
>  return;
> -- 

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


Re: [libav-devel] [PATCH] libx264: Correctly manage constant rate factor params

2014-06-22 Thread Kostya Shishkov
On Sat, Jun 21, 2014 at 12:47:02PM +0200, Luca Barbato wrote:
> By default they are set to -1.
> ---
> 
> Sorry for not checking with the defaults properly.
> 
>  libavcodec/libx264.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index 06e917e..4f44a06 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -179,7 +179,8 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, 
> const AVFrame *frame,
>  x264_encoder_reconfig(x4->enc, &x4->params);
>  }
> 
> -if (x4->params.rc.i_rc_method == X264_RC_CRF &&
> +if (x4->crf >= 0 &&
> +x4->params.rc.i_rc_method == X264_RC_CRF &&
>  x4->params.rc.f_rf_constant != x4->crf) {
>  x4->params.rc.f_rf_constant = x4->crf;
>  x264_encoder_reconfig(x4->enc, &x4->params);
> @@ -191,7 +192,7 @@ static int X264_frame(AVCodecContext *ctx, AVPacket *pkt, 
> const AVFrame *frame,
>  x264_encoder_reconfig(x4->enc, &x4->params);
>  }
> 
> -if (x4->crf_max &&
> +if (x4->crf_max >= 0 &&
>  x4->params.rc.f_rf_constant_max != x4->crf_max) {
>  x4->params.rc.f_rf_constant_max = x4->crf_max;
>  x264_encoder_reconfig(x4->enc, &x4->params);
> --

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


Re: [libav-devel] Indeo4 B-frames decoding support

2014-06-20 Thread Kostya Shishkov
On Fri, Jun 20, 2014 at 08:46:11PM +0200, Dirk Ausserhaus wrote:
> Here is my patch for decoding B-frames in Indeo4 at last. And another
> patch to make things a bit simpler first.

At last!
 
> I suppose I have to give some explanation what I did here and why.
> 
> First of all, every band has three buffers—current, reference and one
> for scalability. I had to add the fourth buffer for the second
> reference and change Indeo4 switch_buffers function to handle the
> second reference frame.
> 
> Second, I had to add the second motion vector to the block
> information. I chose a new name for it in order to have less impact on
> existing code, since making mv_x and mv_y arrays would lead to more
> changes in all places and more possible errors.
> 
> Third, I had to extend ivi_mc function to handle B-frames and add some
> new functions for bidirectional motion compensation. Motion types are
> distinguished by mc_type and mc_type2 variables. Normally they should
> contain fractional offset for the motion compensation but I overloaded
> it so -1 means no motion vector at all.

That sounds reasonable to me.

> I've tested it on various Indeo4 and Indeo5 samples and it doesn't
> seem to break anything.

That's the best part ;)

> From 49d6cf8a6415d2cc49576919aea38a869872fa6b Mon Sep 17 00:00:00 2001
> From: Dirk Ausserhaus 
> Date: Fri, 20 Jun 2014 20:15:20 +0200
> Subject: [PATCH 1/2] indeo45: use is_indeo4 context flag instead of checking
>  codec ID
> 
> ---
>  libavcodec/indeo4.c |2 ++
>  libavcodec/indeo5.c |2 ++
>  libavcodec/ivi_common.c |   10 --
>  libavcodec/ivi_common.h |2 ++
>  4 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/indeo4.c b/libavcodec/indeo4.c
> index 3e97221..6893077 100644
> --- a/libavcodec/indeo4.c
> +++ b/libavcodec/indeo4.c
> @@ -620,6 +620,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
>  ctx->switch_buffers   = switch_buffers;
>  ctx->is_nonnull_frame = is_nonnull_frame;
>  
> +ctx->is_indeo4 = 1;
> +
>  ctx->p_frame = av_frame_alloc();
>  if (!ctx->p_frame)
>  return AVERROR(ENOMEM);
> diff --git a/libavcodec/indeo5.c b/libavcodec/indeo5.c
> index 476355b..2465ce6 100644
> --- a/libavcodec/indeo5.c
> +++ b/libavcodec/indeo5.c
> @@ -640,6 +640,8 @@ static av_cold int decode_init(AVCodecContext *avctx)
>  ctx->switch_buffers   = switch_buffers;
>  ctx->is_nonnull_frame = is_nonnull_frame;
>  
> +ctx->is_indeo4 = 0;
> +
>  avctx->pix_fmt = AV_PIX_FMT_YUV410P;
>  
>  return 0;
> diff --git a/libavcodec/ivi_common.c b/libavcodec/ivi_common.c
> index 6eb5b6c..93936c2 100644
> --- a/libavcodec/ivi_common.c
> +++ b/libavcodec/ivi_common.c
> @@ -968,8 +968,7 @@ int ff_ivi_decode_frame(AVCodecContext *avctx, void 
> *data, int *got_frame,
>  if (ctx->gop_invalid)
>  return AVERROR_INVALIDDATA;
>  
> -if (avctx->codec_id == AV_CODEC_ID_INDEO4 &&
> -ctx->frame_type == IVI4_FRAMETYPE_NULL_LAST) {
> +if (ctx->is_indeo4 && ctx->frame_type == IVI4_FRAMETYPE_NULL_LAST) {
>  if (ctx->got_p_frame) {
>  av_frame_move_ref(data, ctx->p_frame);
>  *got_frame = 1;
> @@ -1027,7 +1026,7 @@ int ff_ivi_decode_frame(AVCodecContext *avctx, void 
> *data, int *got_frame,
>  }
>  
>  if (ctx->is_scalable) {
> -if (avctx->codec_id == AV_CODEC_ID_INDEO4)
> +if (ctx->is_indeo4)
>  ff_ivi_recompose_haar(&ctx->planes[0], frame->data[0], 
> frame->linesize[0]);
>  else
>  ff_ivi_recompose53   (&ctx->planes[0], frame->data[0], 
> frame->linesize[0]);
> @@ -1045,8 +1044,7 @@ int ff_ivi_decode_frame(AVCodecContext *avctx, void 
> *data, int *got_frame,
>   * to be the only way to handle the B-frames mode.
>   * That's exactly the same Intel decoders do.
>   */
> -if (avctx->codec_id == AV_CODEC_ID_INDEO4 &&
> -ctx->frame_type == IVI4_FRAMETYPE_INTRA) {
> +if (ctx->is_indeo4 && ctx->frame_type == IVI4_FRAMETYPE_INTRA) {
>  int left;
>  
>  while (get_bits(&ctx->gb, 8)); // skip version string
> @@ -1077,7 +1075,7 @@ av_cold int ff_ivi_decode_close(AVCodecContext *avctx)
>  ff_free_vlc(&ctx->mb_vlc.cust_tab);
>  
>  #if IVI4_STREAM_ANALYSER
> -if (avctx->codec_id == AV_CODEC_ID_INDEO4) {
> +if (ctx->is_indeo4) {
>  if (ctx->is_scalable)
>  av_log(avctx, AV_LOG_ERROR, "This video uses scalability mode!\n");
>  if (ctx->uses_tiling)
> diff --git a/libavcodec/ivi_common.h b/libavcodec/ivi_common.h
> index 604b549..515b073 100644
> --- a/libavcodec/ivi_common.h
> +++ b/libavcodec/ivi_common.h
> @@ -261,6 +261,8 @@ typedef struct IVI45DecContext {
>  
>  int gop_invalid;
>  
> +int is_indeo4;
> +
>  AVFrame *p_frame;
>  int got_p_frame;
>  } IVI45DecContext;
> -- 
> 1.7.9.5

There's one more place - where quantiser is clipped but context is not passed
there. So LGTM and I'll apply this s

Re: [libav-devel] [PATCH] Silence errors for I263 dummy frame

2014-06-19 Thread Kostya Shishkov
On Wed, Jun 18, 2014 at 10:18:44AM +0200, Dirk Ausserhaus wrote:
> Indeo 4 B-frame takes more time than expected—it involves new buffer
> management since two reference frames are required now, these
> references should be propagated into ivi_common too etc.
> 
> And averaging there is (A + B) >> 1 indeed.
> 
> Meanwhile I've looked at I263 PB-frames decoding. Looks like B-frame
> part of PB-frame is skipped in the middle of P-frame decoding so it
> needs a bit more work too. I must admit current organisation is
> somewhat strange—I263 frame header is read in intelh263dec.c,
> PB-frames are handled in ituh263dec.c but main frame decoding function
> is in h263dec.c.
> 
> Anyway, here's a small patch to silence decoding errors on dummy
> frames. Those frame look like 8 bytes of previous frame header
> (leftover garbage?) with first byte overwritten to contain picture
> number.

No objections so far, so I've checked that it passes FATE and committed.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Silence errors for I263 dummy frame

2014-06-18 Thread Kostya Shishkov
On Wed, Jun 18, 2014 at 10:18:44AM +0200, Dirk Ausserhaus wrote:
> Indeo 4 B-frame takes more time than expected—it involves new buffer
> management since two reference frames are required now, these
> references should be propagated into ivi_common too etc.
> 
> And averaging there is (A + B) >> 1 indeed.

Don't hesistate to ask for help with it - I've fixed some bugs with that
decoder too. But I was (and am) too lazy to add less important missing
features like B-frames or transparency (the latter might be useful for
game engine reimplementations after all).
 
> Meanwhile I've looked at I263 PB-frames decoding. Looks like B-frame
> part of PB-frame is skipped in the middle of P-frame decoding so it
> needs a bit more work too. I must admit current organisation is
> somewhat strange—I263 frame header is read in intelh263dec.c,
> PB-frames are handled in ituh263dec.c but main frame decoding function
> is in h263dec.c.

I've added that skipping code long time ago. Back then it was a monolithic
decoder (IIRC single monstrous h263dec.c) and definitely no possibility for an
additional decoding frame.

> Anyway, here's a small patch to silence decoding errors on dummy
> frames. Those frame look like 8 bytes of previous frame header
> (leftover garbage?) with first byte overwritten to contain picture
> number.

> From 57a41838048195a85777d133272da848064e22bb Mon Sep 17 00:00:00 2001
> From: Dirk Ausserhaus 
> Date: Wed, 18 Jun 2014 10:01:52 +0200
> Subject: [PATCH] i263: skip dummy frames
> 
> Intel i263 codec has special 8-byte dummy frames that should not be decoded,
> so do not even attempt to decode them and skip them instead.
> ---
>  libavcodec/intelh263dec.c |4 
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/libavcodec/intelh263dec.c b/libavcodec/intelh263dec.c
> index 8a0a381..e34da5c 100644
> --- a/libavcodec/intelh263dec.c
> +++ b/libavcodec/intelh263dec.c
> @@ -26,6 +26,10 @@ int ff_intel_h263_decode_picture_header(MpegEncContext *s)
>  {
>  int format;
>  
> +if (get_bits_left(&s->gb) == 64) { /* special dummy frames */
> +return FRAME_SKIPPED;
> +}
> +
>  /* picture header */
>  if (get_bits_long(&s->gb, 22) != 0x20) {
>  av_log(s->avctx, AV_LOG_ERROR, "Bad picture start code\n");
> -- 

looks good to me
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel

Re: [libav-devel] [PATCH] Remove avserver.

2014-06-15 Thread Kostya Shishkov
On Sat, Jun 14, 2014 at 09:54:17PM +0200, Anton Khirnov wrote:
> It has not been properly maintained for years and there is little hope
> of that changing in the future.
> It appears simpler to write a new replacement from scratch than
> unbreaking it.
> ---
>  .gitignore   |1 -
>  Changelog|1 +
>  Makefile |3 +-
>  avserver.c   | 4685 
> --
>  configure|9 +-
>  doc/avserver.conf|  372 
>  doc/avserver.texi|  276 ---
>  doc/general.texi |1 -
>  libavformat/Makefile |2 -
>  libavformat/allformats.c |1 -
>  libavformat/ffm.h|   59 -
>  libavformat/ffmdec.c |  483 -
>  libavformat/ffmenc.c |  249 ---
>  tests/fate/avformat.mak  |1 -
>  tests/fate/seek.mak  |2 -
>  tests/lavf-regression.sh |4 -
>  tests/ref/lavf/ffm   |3 -
>  tests/ref/seek/lavf-ffm  |   53 -
>  18 files changed, 7 insertions(+), 6198 deletions(-)
>  delete mode 100644 avserver.c
>  delete mode 100644 doc/avserver.conf
>  delete mode 100644 doc/avserver.texi
>  delete mode 100644 libavformat/ffm.h
>  delete mode 100644 libavformat/ffmdec.c
>  delete mode 100644 libavformat/ffmenc.c
>  delete mode 100644 tests/ref/lavf/ffm
>  delete mode 100644 tests/ref/seek/lavf-ffm

It always looks good to me
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] tiffenc: fix packet size calculation

2014-06-13 Thread Kostya Shishkov
On Fri, Jun 13, 2014 at 04:33:21PM +0200, Luca Barbato wrote:
> On 13/06/14 16:30, Tristan Matthews wrote:
> > ---
> >  libavcodec/tiffenc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/tiffenc.c b/libavcodec/tiffenc.c
> > index ccfb07c..c40f340 100644
> > --- a/libavcodec/tiffenc.c
> > +++ b/libavcodec/tiffenc.c
> > @@ -290,7 +290,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket 
> > *pkt,
> >  
> >  if (!pkt->data &&
> >  (ret = av_new_packet(pkt,
> > - avctx->width * avctx->height * s->bpp * 2 +
> > + (avctx->width * avctx->height * s->bpp * 2) 
> > >> 3 +
> >   avctx->height * 4 + FF_MIN_BUFFER_SIZE)) < 0) 
> > {
> >  av_log(avctx, AV_LOG_ERROR, "Error getting output packet.\n");
> >  return ret;
> > 
> 
> Seems probably fine assuming bpp is bits per pixel.

would be better to have height * (width * bpp + 7 >> 3) * 2 instead
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] Rename tpel_template.c ---> pel_template.c

2014-06-12 Thread Kostya Shishkov
On Thu, Jun 12, 2014 at 07:09:54AM -0700, Diego Biurrun wrote:
> The new name more accurately describes what the file is about.
> ---
> 
> ... as suggested by Kostya ...
> 
>  libavcodec/h264qpel_template.c | 2 +-
>  libavcodec/hpeldsp.c   | 2 +-
>  libavcodec/{tpel_template.c => pel_template.c} | 6 +++---
>  libavcodec/qpeldsp.c   | 2 +-
>  libavcodec/tpeldsp.c   | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
>  rename libavcodec/{tpel_template.c => pel_template.c} (97%)

LGTM for some reason
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/1] mpegvideo: synchronize AVFrame pointers in ERContext fully

2014-06-11 Thread Kostya Shishkov
On Wed, Jun 11, 2014 at 07:51:35PM +0200, Janne Grunau wrote:
> Since error resilience uses AVFrame pointers instead of references it
> has to copy NULL pointers too. After a codec flush the last/next frame
> pointers in MpegEncContext are NULL and the old pointers remaining in
> ERContext are invalid. Fixes a crash in vlc for android thumbnailer.
> Reported and debugged by Adrien Maglo .
> ---
>  libavcodec/mpegvideo.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/mpegvideo.c b/libavcodec/mpegvideo.c
> index b2fcce2..ca5ee0b 100644
> --- a/libavcodec/mpegvideo.c
> +++ b/libavcodec/mpegvideo.c
> @@ -2473,8 +2473,11 @@ void ff_mpeg_set_erpic(ERPicture *dst, Picture *src)
>  {
>  int i;
>  
> -if (!src)
> +if (!src) {
> + dst->f  = NULL;
> + dst->tf = NULL;
>  return;
> +}
>  
>  dst->f = src->f;
>  dst->tf = &src->tf;
> -- 

looks passable for this code
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 2/3] amr: Restructure (de)muxer to avoid unnecessary ifdefs

2014-06-11 Thread Kostya Shishkov
On Wed, Jun 11, 2014 at 06:08:12AM -0700, Diego Biurrun wrote:
> ---
>  libavformat/amr.c | 26 --
>  1 file changed, 12 insertions(+), 14 deletions(-)

IMO it's horrible when you have to search for codec definitions somewhere
inside the file instead of conventionally looking at the end.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 1/3] Remove some unnecessary CONFIG_FOO_COMPONENT ifdefs

2014-06-11 Thread Kostya Shishkov
On Wed, Jun 11, 2014 at 06:08:11AM -0700, Diego Biurrun wrote:
> The files are only ever compiled if that condition is true.
> ---
>  libavcodec/ac3enc_float.c | 4 
>  libavcodec/eac3enc.c  | 2 --
>  libavformat/dv.c  | 4 +---
>  3 files changed, 1 insertion(+), 9 deletions(-)

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


Re: [libav-devel] Indeo 4 B-frames

2014-06-10 Thread Kostya Shishkov
On Tue, Jun 10, 2014 at 12:20:44PM +0200, Maxim Polijakowski wrote:
> Am 08.06.2014 14:15, schrieb Dirk Ausserhaus:
> >On Sun, Jun 8, 2014 at 2:10 PM, Kostya Shishkov
> > wrote:
> >>On Sun, Jun 08, 2014 at 01:53:49PM +0200, Dirk Ausserhaus wrote:
> >>>Here's a patch that fixes decoding of Indeo 4 B-frames.
> >>Looks correct. I should've done it long time ago and saved you some work.
> >>
> >>>Now the main problem is Indeo 4 B-frames reconstruction. For that I
> >>>need to modify IVI context to store the second pair of vectors (that
> >>>is not hard at all) and make it perform the averaging motion
> >>>compensation for B-blocks (that one seems to require some more
> >>>hacking).
> >>It seems tricker than most codecs since in Indeo one does not add residue to
> >>motion compensation but rather the othe way around so motion function will
> >>have to do averaging inside before adding result to the block.
> 
> IIRC, it performs a simple averaging motion compensation like this:
> 
> (X + Y) / 2.
> 
> IIRC, it doesn't even do a proper rounding.

Should not matter much in this case.
 
> The tricky part was which frames to choose - two different kinds of
> NULL frames (type 6 and 7) play the key role there.
> I'll try to search my data for more information on this topic. Not
> sure how sucessful it will be because alot of time has passed since
> I worked on this codec...

At least in volcano.avi you have a sequence of
I, I, I, IP, B, B, type6 (null_last), type4 (droppable P), IP, B, B, type6

Another file I have features this pattern:
I, I, I, IP, B, B, P, B, B, P

Hopefully Dirk will figure something out.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [RFC/PATCH] Remove avserver.

2014-06-09 Thread Kostya Shishkov
On Mon, Jun 09, 2014 at 02:29:53PM +0200, Anton Khirnov wrote:
> It has not been properly maintained for years and there is little hope
> of that changing in the future.
> It appears simpler to write a new replacement from scratch than
> unbreaking it.

+42

> ---
> Is anyone actually using this for anything serious?
> 
> It interferes with my work and I'd rather not waste time on something we all
> agree is crap and should be removed.

Or have disagreeing people to have their heads checked.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] Indeo 4 B-frames

2014-06-08 Thread Kostya Shishkov
On Sun, Jun 08, 2014 at 01:53:49PM +0200, Dirk Ausserhaus wrote:
> Here's a patch that fixes decoding of Indeo 4 B-frames.

Looks correct. I should've done it long time ago and saved you some work.
 
> Now the main problem is Indeo 4 B-frames reconstruction. For that I
> need to modify IVI context to store the second pair of vectors (that
> is not hard at all) and make it perform the averaging motion
> compensation for B-blocks (that one seems to require some more
> hacking).

It seems tricker than most codecs since in Indeo one does not add residue to
motion compensation but rather the othe way around so motion function will
have to do averaging inside before adding result to the block.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 101/132] bink: Rename BinkDSPContext member so as not to clash with BlockDSPContext

2014-06-02 Thread Kostya Shishkov
On Mon, Jun 02, 2014 at 04:34:34AM -0700, Diego Biurrun wrote:
> ---
>  libavcodec/bink.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/libavcodec/bink.c b/libavcodec/bink.c
> index e1312a8..d1e94d9 100644
> --- a/libavcodec/bink.c
> +++ b/libavcodec/bink.c
> @@ -115,7 +115,7 @@ typedef struct BinkContext {
>  AVCodecContext *avctx;
>  DSPContext dsp;
>  HpelDSPContext hdsp;
> -BinkDSPContext bdsp;
> +BinkDSPContext binkdsp;
>  AVFrame*last;
>  intversion;  ///< internal Bink file version
>  inthas_alpha;

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


Re: [libav-devel] Fix Indeo4 IP-frames

2014-05-31 Thread Kostya Shishkov
On Fri, May 30, 2014 at 02:38:05PM +0200, Dirk Ausserhaus wrote:
> On Fri, May 30, 2014 at 7:38 AM, Anton Khirnov  wrote:
> >
> >
> > On Thu, 29 May 2014 13:44:34 +0200, Dirk Ausserhaus  
> > wrote:
> > > diff --git a/libavcodec/ivi_common.c b/libavcodec/ivi_common.c
> > > index cf4df18..ef4df10 100644
> > > --- a/libavcodec/ivi_common.c
> > > +++ b/libavcodec/ivi_common.c
> > > @@ -968,6 +968,21 @@ int ff_ivi_decode_frame(AVCodecContext *avctx, void 
> > > *data, int *got_frame,
> > >  if (ctx->gop_invalid)
> > >  return AVERROR_INVALIDDATA;
> > >
> > > +if (avctx->codec_id == AV_CODEC_ID_INDEO4 &&
> > > +ctx->frame_type == IVI4_FRAMETYPE_NULL_LAST) {
> > > +if (ctx->got_p_frame) {
> > > +if ((result = av_frame_ref(data, ctx->p_frame)) < 0)
> > > +return result;
> > > +av_frame_free(&ctx->p_frame);
> > > +ctx->p_frame = av_frame_alloc();
> >
> > The above 4 lines should be replaced with av_frame_move_ref(data, 
> > ctx->p_frame)
> >
> > The rest looks fine to me.
> 
> 
> Here're the updated patches in case you've not committed them yet.

Now they are committed, thanks.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] matroskaenc: Allow VP9 and Opus in webm

2014-05-31 Thread Kostya Shishkov
On Sat, May 31, 2014 at 09:23:37AM +0200, Anton Khirnov wrote:
> From: Tudor Suciu 
> 
> Signed-off-by: Anton Khirnov 
> ---
> The webm official page does not mention that, but Google seems to claim those
> are now offically supported. Nice to see they are keeping all the matroska
> traditions.
> ---
>  libavformat/matroskaenc.c |4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index f6af0f8..919cdfd 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -670,9 +670,11 @@ static int mkv_write_tracks(AVFormatContext *s)
>  }
>  
>  if (mkv->mode == MODE_WEBM && !(codec->codec_id == AV_CODEC_ID_VP8 ||
> +codec->codec_id == AV_CODEC_ID_VP9 ||
> +codec->codec_id == AV_CODEC_ID_OPUS 
> ||
>  codec->codec_id == 
> AV_CODEC_ID_VORBIS)) {
>  av_log(s, AV_LOG_ERROR,
> -   "Only VP8 video and Vorbis audio are supported for 
> WebM.\n");
> +   "Only VP8 or VP9 video and Vorbis or Opus audio are 
> supported for WebM.\n");
>  return AVERROR(EINVAL);
>  }
>  
> -- 

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


Re: [libav-devel] [PATCH] Remove all Blackfin architecture optimizations

2014-05-30 Thread Kostya Shishkov
On Fri, May 30, 2014 at 03:18:59PM +0200, Luca Barbato wrote:
> On 30/05/14 14:46, Kostya Shishkov wrote:
> > On Fri, May 30, 2014 at 05:25:18AM -0700, Diego Biurrun wrote:
> >> Blackfin is a painful platform to work with, no test machines are available
> >> and the range of multimedia applications is dubious. Thus it only 
> >> represents
> >> a maintenance burden.
> >> ---
> >>
> >> As discussed previously at FOSDEM/VDD and on this mailing list ...
> >>
> >>  libavcodec/bfin/Makefile |  10 -
> >>  libavcodec/bfin/dsputil.S| 382 --
> >>  libavcodec/bfin/dsputil_init.c   | 195 
> >>  libavcodec/bfin/fdct_bfin.S  | 325 ---
> >>  libavcodec/bfin/hpel_pixels_no_rnd.S |  81 -
> >>  libavcodec/bfin/hpeldsp_init.c   | 146 -
> >>  libavcodec/bfin/idct_bfin.S  | 297 -
> >>  libavcodec/bfin/mathops.h|  44 ---
> >>  libavcodec/bfin/pixels.S | 207 
> >>  libavcodec/bfin/pixels.h |  40 ---
> >>  libavcodec/bfin/vp3dsp.S | 273 
> >>  libavcodec/bfin/vp3dsp_init.c|  66 
> >>  libavcodec/dct-test.c|  12 -
> >>  libavcodec/dsputil.c |   2 -
> >>  libavcodec/dsputil.h |   2 -
> >>  libavcodec/hpeldsp.c |   2 -
> >>  libavcodec/hpeldsp.h |   1 -
> >>  libavcodec/vp3dsp.c  |   2 -
> >>  libavcodec/vp3dsp.h  |   1 -
> >>  libavutil/bfin/asm.h |  54 
> >>  libavutil/bfin/attributes.h  |  34 --
> >>  libswscale/bfin/Makefile |   3 -
> >>  libswscale/bfin/internal_bfin.S  | 599 
> >> ---
> >>  libswscale/bfin/swscale_bfin.c   |  84 -
> >>  libswscale/bfin/yuv2rgb_bfin.c   | 197 
> >>  libswscale/swscale.h |   2 +
> >>  libswscale/swscale_internal.h|  16 -
> >>  libswscale/swscale_unscaled.c|   2 -
> >>  libswscale/version.h |   3 +
> >>  libswscale/yuv2rgb.c |   2 -
> >>  30 files changed, 5 insertions(+), 3079 deletions(-)
> >>  delete mode 100644 libavcodec/bfin/Makefile
> >>  delete mode 100644 libavcodec/bfin/dsputil.S
> >>  delete mode 100644 libavcodec/bfin/dsputil_init.c
> >>  delete mode 100644 libavcodec/bfin/fdct_bfin.S
> >>  delete mode 100644 libavcodec/bfin/hpel_pixels_no_rnd.S
> >>  delete mode 100644 libavcodec/bfin/hpeldsp_init.c
> >>  delete mode 100644 libavcodec/bfin/idct_bfin.S
> >>  delete mode 100644 libavcodec/bfin/mathops.h
> >>  delete mode 100644 libavcodec/bfin/pixels.S
> >>  delete mode 100644 libavcodec/bfin/pixels.h
> >>  delete mode 100644 libavcodec/bfin/vp3dsp.S
> >>  delete mode 100644 libavcodec/bfin/vp3dsp_init.c
> >>  delete mode 100644 libavutil/bfin/asm.h
> >>  delete mode 100644 libavutil/bfin/attributes.h
> >>  delete mode 100644 libswscale/bfin/Makefile
> >>  delete mode 100644 libswscale/bfin/internal_bfin.S
> >>  delete mode 100644 libswscale/bfin/swscale_bfin.c
> >>  delete mode 100644 libswscale/bfin/yuv2rgb_bfin.c
> > 
> > LGTM
> 
> +1 Please add to the website a mention that if somebody is willing to
> provide hardware to keep it maintained we'll be happy to re-enable its
> support.

DECENT hardware, not the one where you have to put strings into special
section just to make hello.c run
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] Remove all Blackfin architecture optimizations

2014-05-30 Thread Kostya Shishkov
On Fri, May 30, 2014 at 05:25:18AM -0700, Diego Biurrun wrote:
> Blackfin is a painful platform to work with, no test machines are available
> and the range of multimedia applications is dubious. Thus it only represents
> a maintenance burden.
> ---
> 
> As discussed previously at FOSDEM/VDD and on this mailing list ...
> 
>  libavcodec/bfin/Makefile |  10 -
>  libavcodec/bfin/dsputil.S| 382 --
>  libavcodec/bfin/dsputil_init.c   | 195 
>  libavcodec/bfin/fdct_bfin.S  | 325 ---
>  libavcodec/bfin/hpel_pixels_no_rnd.S |  81 -
>  libavcodec/bfin/hpeldsp_init.c   | 146 -
>  libavcodec/bfin/idct_bfin.S  | 297 -
>  libavcodec/bfin/mathops.h|  44 ---
>  libavcodec/bfin/pixels.S | 207 
>  libavcodec/bfin/pixels.h |  40 ---
>  libavcodec/bfin/vp3dsp.S | 273 
>  libavcodec/bfin/vp3dsp_init.c|  66 
>  libavcodec/dct-test.c|  12 -
>  libavcodec/dsputil.c |   2 -
>  libavcodec/dsputil.h |   2 -
>  libavcodec/hpeldsp.c |   2 -
>  libavcodec/hpeldsp.h |   1 -
>  libavcodec/vp3dsp.c  |   2 -
>  libavcodec/vp3dsp.h  |   1 -
>  libavutil/bfin/asm.h |  54 
>  libavutil/bfin/attributes.h  |  34 --
>  libswscale/bfin/Makefile |   3 -
>  libswscale/bfin/internal_bfin.S  | 599 
> ---
>  libswscale/bfin/swscale_bfin.c   |  84 -
>  libswscale/bfin/yuv2rgb_bfin.c   | 197 
>  libswscale/swscale.h |   2 +
>  libswscale/swscale_internal.h|  16 -
>  libswscale/swscale_unscaled.c|   2 -
>  libswscale/version.h |   3 +
>  libswscale/yuv2rgb.c |   2 -
>  30 files changed, 5 insertions(+), 3079 deletions(-)
>  delete mode 100644 libavcodec/bfin/Makefile
>  delete mode 100644 libavcodec/bfin/dsputil.S
>  delete mode 100644 libavcodec/bfin/dsputil_init.c
>  delete mode 100644 libavcodec/bfin/fdct_bfin.S
>  delete mode 100644 libavcodec/bfin/hpel_pixels_no_rnd.S
>  delete mode 100644 libavcodec/bfin/hpeldsp_init.c
>  delete mode 100644 libavcodec/bfin/idct_bfin.S
>  delete mode 100644 libavcodec/bfin/mathops.h
>  delete mode 100644 libavcodec/bfin/pixels.S
>  delete mode 100644 libavcodec/bfin/pixels.h
>  delete mode 100644 libavcodec/bfin/vp3dsp.S
>  delete mode 100644 libavcodec/bfin/vp3dsp_init.c
>  delete mode 100644 libavutil/bfin/asm.h
>  delete mode 100644 libavutil/bfin/attributes.h
>  delete mode 100644 libswscale/bfin/Makefile
>  delete mode 100644 libswscale/bfin/internal_bfin.S
>  delete mode 100644 libswscale/bfin/swscale_bfin.c
>  delete mode 100644 libswscale/bfin/yuv2rgb_bfin.c

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


Re: [libav-devel] Fix Indeo4 IP-frames

2014-05-30 Thread Kostya Shishkov
On Fri, May 30, 2014 at 02:38:05PM +0200, Dirk Ausserhaus wrote:
> On Fri, May 30, 2014 at 7:38 AM, Anton Khirnov  wrote:
> >
> >
> > On Thu, 29 May 2014 13:44:34 +0200, Dirk Ausserhaus  
> > wrote:
> > > diff --git a/libavcodec/ivi_common.c b/libavcodec/ivi_common.c
> > > index cf4df18..ef4df10 100644
> > > --- a/libavcodec/ivi_common.c
> > > +++ b/libavcodec/ivi_common.c
> > > @@ -968,6 +968,21 @@ int ff_ivi_decode_frame(AVCodecContext *avctx, void 
> > > *data, int *got_frame,
> > >  if (ctx->gop_invalid)
> > >  return AVERROR_INVALIDDATA;
> > >
> > > +if (avctx->codec_id == AV_CODEC_ID_INDEO4 &&
> > > +ctx->frame_type == IVI4_FRAMETYPE_NULL_LAST) {
> > > +if (ctx->got_p_frame) {
> > > +if ((result = av_frame_ref(data, ctx->p_frame)) < 0)
> > > +return result;
> > > +av_frame_free(&ctx->p_frame);
> > > +ctx->p_frame = av_frame_alloc();
> >
> > The above 4 lines should be replaced with av_frame_move_ref(data, 
> > ctx->p_frame)
> >
> > The rest looks fine to me.
> 
> 
> Here're the updated patches in case you've not committed them yet.
> 
> P.S. I might look at I263 too. What are the samples with IP-frames there?

Thank you very much.
I've misremembered - I263 has PB-frames which are essentially the same things.
Files like
https://samples.libav.org/A-codecs/IMC/mensaje.avi
https://samples.libav.org/A-codecs/IMC/neal73_saber.avi or
https://samples.libav.org/A-codecs/IMC/nyc.avi
contain them. But decoder is relying on dreaded MpegEncContext and it might be
somewhat tricky to find where to hack it.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] Fix Indeo4 IP-frames

2014-05-30 Thread Kostya Shishkov
On Thu, May 29, 2014 at 01:44:34PM +0200, Dirk Ausserhaus wrote:
> Hello,
> I want to improve Indeo4 decoder to be able to watch old samples I have.
> The problem with files like
> http://samples.libav.org/V-codecs/IV41/indeo4-avi/volcano.avi is that they
> have IP-frames with both I and P frame in the same packet. I've made indeo4
> decoder decode that second frame and now I get less garbage frames with it.

Hmm, I wonder if the same approach should be used to decode Intel H.263 in
AVI, it's the same PB-frames after all (but it's MpegEncContext...)
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH] mmst: K&R formatting cosmetics

2014-05-28 Thread Kostya Shishkov
On Wed, May 28, 2014 at 04:57:25PM +0200, Luca Barbato wrote:
> ---
> 
> While investigating a bug report...

Do we now accept bugreports for source code being badly formatted?
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 095/132] dsputil: Move SVQ1 encoding specific bits into svq1enc

2014-05-28 Thread Kostya Shishkov
On Wed, May 28, 2014 at 12:06:16PM +0200, Diego Biurrun wrote:
> On Tue, May 27, 2014 at 06:16:28PM +0200, Kostya Shishkov wrote:
> > On Tue, May 27, 2014 at 08:22:29AM -0700, Diego Biurrun wrote:
> > > ---
> > >  libavcodec/dsputil.c | 12 --
> > >  libavcodec/dsputil.h |  3 --
> > >  libavcodec/ppc/Makefile  |  1 +
> > >  libavcodec/ppc/int_altivec.c | 48 +---
> > >  libavcodec/ppc/svq1enc_altivec.c | 80 
> > > 
> > >  libavcodec/svq1enc.c | 58 ++---
> > >  libavcodec/svq1enc.h | 78 
> > > +++
> > >  libavcodec/x86/Makefile  |  1 +
> > >  libavcodec/x86/dsputilenc_mmx.c  | 36 --
> > >  libavcodec/x86/svq1enc_mmx.c | 72 
> > > 
> > >  libavutil/ppc/util_altivec.h |  3 ++
> > >  11 files changed, 255 insertions(+), 137 deletions(-)
> > >  create mode 100644 libavcodec/ppc/svq1enc_altivec.c
> > >  create mode 100644 libavcodec/svq1enc.h
> > >  create mode 100644 libavcodec/x86/svq1enc_mmx.c
> > > 
> > [...]
> > > diff --git a/libavcodec/ppc/int_altivec.c b/libavcodec/ppc/int_altivec.c
> > > index 42c396c..fa3cb66 100644
> > > --- a/libavcodec/ppc/int_altivec.c
> > > +++ b/libavcodec/ppc/int_altivec.c
> > > @@ -30,54 +30,10 @@
> > >  
> > >  #include "libavutil/attributes.h"
> > >  #include "libavutil/ppc/types_altivec.h"
> > > +#include "libavutil/ppc/util_altivec.h"
> > 
> > somehow this looks like a stray include
> 
> I move the vec_unaligned_load macro to that header file.  Shall I do it
> as a separate step?

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


Re: [libav-devel] [PATCH 098/132] dsputil: Move APE-specific bits into apedsp

2014-05-27 Thread Kostya Shishkov
On Tue, May 27, 2014 at 07:24:18PM +0200, Diego Biurrun wrote:
> On Tue, May 27, 2014 at 06:20:01PM +0200, Kostya Shishkov wrote:
> > On Tue, May 27, 2014 at 08:22:32AM -0700, Diego Biurrun wrote:
> > > --- a/libavcodec/apedec.c
> > > +++ b/libavcodec/apedec.c
> > > @@ -193,8 +195,6 @@ static void predictor_decode_stereo_3930(APEContext 
> > > *ctx, int count);
> > >  
> > > -// TODO: dsputilize
> > > -
> > >  static av_cold int ape_decode_close(AVCodecContext *avctx)
> > >  {
> > >  APEContext *s = avctx->priv_data;
> > 
> > Damn, I still want dsputilized ape_decode_close()
> 
> What's stopping you, you lazy lout? ;-p

My laziness of course!
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 100/132] dsputil: Split off quarterpel bits into their own context

2014-05-27 Thread Kostya Shishkov
On Tue, May 27, 2014 at 08:22:34AM -0700, Diego Biurrun wrote:
> ---
>  configure  |   13 +-
>  libavcodec/Makefile|1 +
>  libavcodec/cavs.c  |1 +
>  libavcodec/cavsdsp.h   |4 +-
>  libavcodec/dsputil.c   |  727 -
>  libavcodec/dsputil.h   |   37 -
>  libavcodec/h263dec.c   |   16 +-
>  libavcodec/h264.c  |1 +
>  libavcodec/h264qpel.h  |2 +-
>  libavcodec/motion_est.c|   16 +-
>  libavcodec/mpegvideo.c |1 +
>  libavcodec/mpegvideo.h |2 +
>  libavcodec/mpegvideo_enc.c |9 +-
>  libavcodec/mpegvideo_motion.c  |1 +
>  libavcodec/mss2.c  |7 +-
>  libavcodec/{dsputil_template.c => qpel_template.c} |8 +-
>  libavcodec/{dsputil.c => qpeldsp.c}| 1562 
> +---
>  libavcodec/qpeldsp.h   |   78 +
>  libavcodec/rv34.c  |1 +
>  libavcodec/rv34dsp.h   |2 +-
>  libavcodec/vc1dec.c|5 +-
>  libavcodec/vc1dsp.c|2 +-
>  libavcodec/wmv2dsp.h   |2 +-
>  libavcodec/x86/Makefile|   11 +-
>  libavcodec/x86/dsputil_init.c  |  464 --
>  libavcodec/x86/{mpeg4qpel.asm => qpeldsp.asm}  |3 +-
>  libavcodec/x86/{dsputil_init.c => qpeldsp_init.c}  |  179 +--
>  27 files changed, 167 insertions(+), 2988 deletions(-)
>  rename libavcodec/{dsputil_template.c => qpel_template.c} (98%)
>  copy libavcodec/{dsputil.c => qpeldsp.c} (52%)
>  create mode 100644 libavcodec/qpeldsp.h
>  rename libavcodec/x86/{mpeg4qpel.asm => qpeldsp.asm} (99%)
>  copy libavcodec/x86/{dsputil_init.c => qpeldsp_init.c} (84%)

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


Re: [libav-devel] [PATCH 099/132] dsputil: Move bink-specific add_pixels8 to binkdsp

2014-05-27 Thread Kostya Shishkov
On Tue, May 27, 2014 at 08:22:33AM -0700, Diego Biurrun wrote:
> ---
>  libavcodec/bink.c|  4 ++--
>  libavcodec/binkdsp.c | 21 +
>  libavcodec/binkdsp.h |  1 +
>  libavcodec/dsputil.c | 21 -
>  libavcodec/dsputil.h |  1 -
>  5 files changed, 24 insertions(+), 24 deletions(-)

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


Re: [libav-devel] [PATCH 098/132] dsputil: Move APE-specific bits into apedsp

2014-05-27 Thread Kostya Shishkov
On Tue, May 27, 2014 at 08:22:32AM -0700, Diego Biurrun wrote:
> ---
>  libavcodec/apedec.c|  33 +++-
>  libavcodec/apedsp.h|  44 ++
>  libavcodec/arm/Makefile|   2 +
>  libavcodec/arm/apedsp_init_arm.c   |  38 +
>  libavcodec/arm/{int_neon.S => apedsp_neon.S}   |  28 
>  libavcodec/arm/dsputil_init_neon.c |   5 -
>  libavcodec/arm/int_neon.S  |  40 -
>  libavcodec/dsputil.c   |  15 --
>  libavcodec/dsputil.h   |  10 --
>  libavcodec/ppc/Makefile|   1 +
>  libavcodec/ppc/{int_altivec.c => apedsp_altivec.c} |  39 +
>  libavcodec/ppc/int_altivec.c   |  42 --
>  libavcodec/x86/Makefile|   2 +
>  libavcodec/x86/apedsp.asm  | 167 
> +
>  libavcodec/x86/apedsp_init.c   |  49 ++
>  libavcodec/x86/dsputil.asm | 137 -
>  libavcodec/x86/dsputil_init.c  |  13 --
>  17 files changed, 337 insertions(+), 328 deletions(-)
>  create mode 100644 libavcodec/apedsp.h
>  create mode 100644 libavcodec/arm/apedsp_init_arm.c
>  copy libavcodec/arm/{int_neon.S => apedsp_neon.S} (70%)
>  copy libavcodec/ppc/{int_altivec.c => apedsp_altivec.c} (74%)
>  create mode 100644 libavcodec/x86/apedsp.asm
>  create mode 100644 libavcodec/x86/apedsp_init.c
> 
> diff --git a/libavcodec/apedec.c b/libavcodec/apedec.c
> index 8669db8..f45693a 100644
> --- a/libavcodec/apedec.c
> +++ b/libavcodec/apedec.c
> @@ -23,6 +23,7 @@
>  #include "libavutil/avassert.h"
>  #include "libavutil/channel_layout.h"
>  #include "libavutil/opt.h"
> +#include "apedsp.h"
>  #include "avcodec.h"
>  #include "dsputil.h"
>  #include "bytestream.h"
> @@ -134,6 +135,7 @@ typedef struct APEContext {
>  AVClass *class;  ///< class for AVOptions
>  AVCodecContext *avctx;
>  DSPContext dsp;
> +APEDSPContext adsp;
>  int channels;
>  int samples; ///< samples left to decode in 
> current frame
>  int bps;
> @@ -193,8 +195,6 @@ static void predictor_decode_stereo_3930(APEContext *ctx, 
> int count);
>  static void predictor_decode_mono_3950(APEContext *ctx, int count);
>  static void predictor_decode_stereo_3950(APEContext *ctx, int count);
>  
> -// TODO: dsputilize
> -
>  static av_cold int ape_decode_close(AVCodecContext *avctx)
>  {
>  APEContext *s = avctx->priv_data;

Damn, I still want dsputilized ape_decode_close()

> @@ -210,6 +210,19 @@ static av_cold int ape_decode_close(AVCodecContext 
> *avctx)
>  return 0;
>  }
>  
> +static int32_t scalarproduct_and_madd_int16_c(int16_t *v1, const int16_t *v2,
> +  const int16_t *v3,
> +  int order, int mul)
> +{
> +int res = 0;
> +
> +while (order--) {
> +res   += *v1 * *v2++;
> +*v1++ += mul * *v3++;
> +}
> +return res;
> +}
> +
>  static av_cold int ape_decode_init(AVCodecContext *avctx)
>  {
>  APEContext *s = avctx->priv_data;
> @@ -290,6 +303,15 @@ static av_cold int ape_decode_init(AVCodecContext *avctx)
>  s->predictor_decode_stereo = predictor_decode_stereo_3950;
>  }
>  
> +s->adsp.scalarproduct_and_madd_int16 = scalarproduct_and_madd_int16_c;
> +
> +if (ARCH_ARM)
> +ff_apedsp_init_arm(&s->adsp);
> +if (ARCH_PPC)
> +ff_apedsp_init_ppc(&s->adsp);
> +if (ARCH_X86)
> +ff_apedsp_init_x86(&s->adsp);
> +
>  ff_dsputil_init(&s->dsp, avctx);
>  avctx->channel_layout = (avctx->channels==2) ? AV_CH_LAYOUT_STEREO : 
> AV_CH_LAYOUT_MONO;
>  
> @@ -1261,9 +1283,10 @@ static void do_apply_filter(APEContext *ctx, int 
> version, APEFilter *f,
>  
>  while (count--) {
>  /* round fixedpoint scalar product */
> -res = ctx->dsp.scalarproduct_and_madd_int16(f->coeffs, f->delay - 
> order,
> -f->adaptcoeffs - order,
> -order, APESIGN(*data));
> +res = ctx->adsp.scalarproduct_and_madd_int16(f->coeffs,
> + f->delay - order,
> + f->adaptcoeffs - order,
> + order, APESIGN(*data));
>  res = (res + (1 << (fracbits - 1))) >> fracbits;
>  res += *data;
>  *data++ = res;

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


Re: [libav-devel] [PATCH 097/132] dsputil: Move mspel_pixels_tab to the only place it is used

2014-05-27 Thread Kostya Shishkov
On Tue, May 27, 2014 at 08:22:31AM -0700, Diego Biurrun wrote:
> ---
>  libavcodec/dsputil.c | 118 ++
>  libavcodec/dsputil.h |   5 ++-
>  libavcodec/wmv2.c|   8 ++--
>  libavcodec/wmv2dsp.c | 119 
> +++
>  libavcodec/wmv2dsp.h |   4 ++
>  5 files changed, 135 insertions(+), 119 deletions(-)

looks fine on a quick glance
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 095/132] dsputil: Move SVQ1 encoding specific bits into svq1enc

2014-05-27 Thread Kostya Shishkov
On Tue, May 27, 2014 at 08:22:29AM -0700, Diego Biurrun wrote:
> ---
>  libavcodec/dsputil.c | 12 --
>  libavcodec/dsputil.h |  3 --
>  libavcodec/ppc/Makefile  |  1 +
>  libavcodec/ppc/int_altivec.c | 48 +---
>  libavcodec/ppc/svq1enc_altivec.c | 80 
> 
>  libavcodec/svq1enc.c | 58 ++---
>  libavcodec/svq1enc.h | 78 +++
>  libavcodec/x86/Makefile  |  1 +
>  libavcodec/x86/dsputilenc_mmx.c  | 36 --
>  libavcodec/x86/svq1enc_mmx.c | 72 
>  libavutil/ppc/util_altivec.h |  3 ++
>  11 files changed, 255 insertions(+), 137 deletions(-)
>  create mode 100644 libavcodec/ppc/svq1enc_altivec.c
>  create mode 100644 libavcodec/svq1enc.h
>  create mode 100644 libavcodec/x86/svq1enc_mmx.c
> 
[...]
> diff --git a/libavcodec/ppc/int_altivec.c b/libavcodec/ppc/int_altivec.c
> index 42c396c..fa3cb66 100644
> --- a/libavcodec/ppc/int_altivec.c
> +++ b/libavcodec/ppc/int_altivec.c
> @@ -30,54 +30,10 @@
>  
>  #include "libavutil/attributes.h"
>  #include "libavutil/ppc/types_altivec.h"
> +#include "libavutil/ppc/util_altivec.h"

somehow this looks like a stray include

>  #include "libavcodec/dsputil.h"
>  #include "dsputil_altivec.h"
> 
[...]

Moving SVQ1EncContext to a header file was a bit surprising too but in general
patch might be OK.
___
libav-devel mailing list
libav-devel@libav.org
https://lists.libav.org/mailman/listinfo/libav-devel


Re: [libav-devel] [PATCH 096/132] dsputil: Move ff_alternate_*_scan tables to mpegvideo

2014-05-27 Thread Kostya Shishkov
On Tue, May 27, 2014 at 08:22:30AM -0700, Diego Biurrun wrote:
> ---
>  libavcodec/dsputil.c   | 22 --
>  libavcodec/dsputil.h   |  4 
>  libavcodec/mpegvideo.c | 22 ++
>  libavcodec/mpegvideo.h |  4 
>  4 files changed, 26 insertions(+), 26 deletions(-)

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


Re: [libav-devel] [PATCH 094/132] svq1enc: Rename SVQ1Context to SVQ1EncContext

2014-05-27 Thread Kostya Shishkov
On Tue, May 27, 2014 at 08:22:28AM -0700, Diego Biurrun wrote:
> This allows making it visible without name clashes.
> ---
>  libavcodec/svq1enc.c | 20 ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)

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


  1   2   3   4   5   6   7   8   9   10   >