Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2018-01-01 Thread Carl Eugen Hoyos 
2017-12-31 18:45 GMT+01:00 Derek Buitenhuis :
> On 12/31/2017 2:04 PM, Carl Eugen Hoyos wrote:
>> This is not helpful;-(
>> Is it so unlikely that the patch has small gain for gbr
>> (theoretically compensated by existing fast conversion
>> from gbr to rgb) but large impact for rgb (with slow
>> conversion from rgb into gbr)?
>
> I went and tested the speed difference with and without this
> patch, with rgb24 input. You'll be happy to know it came out
> to be pretty much exactly the same speed after averaging 100
> runs of each.

(Why?)

Thank you for the testing!

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Paul B Mahol 
On 12/31/17, Derek Buitenhuis  wrote:
> On 12/31/2017 2:08 PM, Paul B Mahol wrote:
> Why do they change?
> Do I understand correctly that the files get bigger (~5%)?
> If yes, wouldn't that indicate that the patch is not a good idea?

 Its because of different scaling path. Have nothing to do with
 good or bad idea.
>>>
>>> Thank you, imo this indicates the utvideo rgb tests
>>> should be fixed to use rgb input.
>>
>> Patch welcome.
>
> Shouldn't it be easy to force it in utvideo.mak? That would
> avoid the unnecessary FATE changes totally, and IMO, is preferable
> to changing them back later.

No because its swscale nonsense.

If input is rgb, output is unchanged with this patch.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Derek Buitenhuis 
On 12/31/2017 2:04 PM, Carl Eugen Hoyos wrote:
> This is not helpful;-(
> Is it so unlikely that the patch has small gain for gbr
> (theoretically compensated by existing fast conversion
> from gbr to rgb) but large impact for rgb (with slow
> conversion from rgb into gbr)?

I went and tested the speed difference with and without this
patch, with rgb24 input. You'll be happy to know it came out
to be pretty much exactly the same speed after averaging 100
runs of each.

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Derek Buitenhuis 
On 12/31/2017 2:08 PM, Paul B Mahol wrote:
 Why do they change?
 Do I understand correctly that the files get bigger (~5%)?
 If yes, wouldn't that indicate that the patch is not a good idea?
>>>
>>> Its because of different scaling path. Have nothing to do with
>>> good or bad idea.
>>
>> Thank you, imo this indicates the utvideo rgb tests
>> should be fixed to use rgb input.
> 
> Patch welcome.

Shouldn't it be easy to force it in utvideo.mak? That would
avoid the unnecessary FATE changes totally, and IMO, is preferable
to changing them back later.

- Derek

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

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Derek Buitenhuis 
On 12/31/2017 2:21 PM, Carl Eugen Hoyos wrote:
> The real-world issue I see is screen-recording.
> 
> Given that these are small functions and the obvious user advantage
> I really believe supporting both pix_fmts is the best solution.

Generic RGB packing has no place inside the encoder and decoder, IMO.
We've decided against it several times in the past, as well, such as
the cinepak encoder.

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Carl Eugen Hoyos 
2017-12-31 15:16 GMT+01:00 Paul B Mahol :
> On 12/31/17, Carl Eugen Hoyos  wrote:
>> 2017-12-31 10:48 GMT+01:00 Paul B Mahol :
>>
>>>  static void mangle_rgb_planes(uint8_t *dst[4], ptrdiff_t dst_stride,
>>
>> Why don't you add a new function mangle_gbr_planes() and
>> keep rgb encoding? The function is not very large.
>
> No. UtVideo expect planar rgb and planar rgb should be given to it.

The real-world issue I see is screen-recording.

Given that these are small functions and the obvious user advantage
I really believe supporting both pix_fmts is the best solution.

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Paul B Mahol 
On 12/31/17, Carl Eugen Hoyos  wrote:
> 2017-12-31 10:48 GMT+01:00 Paul B Mahol :
>
>>  static void mangle_rgb_planes(uint8_t *dst[4], ptrdiff_t dst_stride,
>
> Why don't you add a new function mangle_gbr_planes() and
> keep rgb encoding? The function is not very large.

No. UtVideo expect planar rgb and planar rgb should be given to it.

This is also to keep sync with decoder, which dropped rgb packed support.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Carl Eugen Hoyos 
2017-12-31 10:48 GMT+01:00 Paul B Mahol :

>  static void mangle_rgb_planes(uint8_t *dst[4], ptrdiff_t dst_stride,

Why don't you add a new function mangle_gbr_planes() and
keep rgb encoding? The function is not very large.

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Paul B Mahol 
On 12/31/17, Carl Eugen Hoyos  wrote:
> 2017-12-31 14:49 GMT+01:00 Paul B Mahol :
>> On 12/31/17, Carl Eugen Hoyos  wrote:
>>> 2017-12-31 10:48 GMT+01:00 Paul B Mahol :
 Signed-off-by: Paul B Mahol 
 ---
  libavcodec/utvideoenc.c   |  47 +---
  tests/ref/fate/utvideoenc_rgb_left| 100
 +-
  tests/ref/fate/utvideoenc_rgb_median  | 100
 +-
  tests/ref/fate/utvideoenc_rgb_none| 100
 +-
  tests/ref/fate/utvideoenc_rgba_left   | 100
 +-
  tests/ref/fate/utvideoenc_rgba_median | 100
 +-
  tests/ref/fate/utvideoenc_rgba_none   | 100
 +-
  7 files changed, 327 insertions(+), 320 deletions(-)
>>>
>>> Is there a speed impact?
>>> (Or actually: How much faster is gbr encoding, how much
>>> slower rgb encoding?)
>>
>> Very very fast, very very slow.
>
> This is not helpful;-(
> Is it so unlikely that the patch has small gain for gbr
> (theoretically compensated by existing fast conversion
> from gbr to rgb) but large impact for rgb (with slow
> conversion from rgb into gbr)?

No.

>
 diff --git a/tests/ref/fate/utvideoenc_rgb_left
 b/tests/ref/fate/utvideoenc_rgb_left
 index a1d200096a..1ee7c58564 100644
 --- a/tests/ref/fate/utvideoenc_rgb_left
 +++ b/tests/ref/fate/utvideoenc_rgb_left
>>>
>>> Why do they change?
>>> Do I understand correctly that the files get bigger (~5%)?
>>> If yes, wouldn't that indicate that the patch is not a good idea?
>>
>> Its because of different scaling path. Have nothing to do with
>> good or bad idea.
>
> Thank you, imo this indicates the utvideo rgb tests
> should be fixed to use rgb input.

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Carl Eugen Hoyos 
2017-12-31 14:49 GMT+01:00 Paul B Mahol :
> On 12/31/17, Carl Eugen Hoyos  wrote:
>> 2017-12-31 10:48 GMT+01:00 Paul B Mahol :
>>> Signed-off-by: Paul B Mahol 
>>> ---
>>>  libavcodec/utvideoenc.c   |  47 +---
>>>  tests/ref/fate/utvideoenc_rgb_left| 100
>>> +-
>>>  tests/ref/fate/utvideoenc_rgb_median  | 100
>>> +-
>>>  tests/ref/fate/utvideoenc_rgb_none| 100
>>> +-
>>>  tests/ref/fate/utvideoenc_rgba_left   | 100
>>> +-
>>>  tests/ref/fate/utvideoenc_rgba_median | 100
>>> +-
>>>  tests/ref/fate/utvideoenc_rgba_none   | 100
>>> +-
>>>  7 files changed, 327 insertions(+), 320 deletions(-)
>>
>> Is there a speed impact?
>> (Or actually: How much faster is gbr encoding, how much
>> slower rgb encoding?)
>
> Very very fast, very very slow.

This is not helpful;-(
Is it so unlikely that the patch has small gain for gbr
(theoretically compensated by existing fast conversion
from gbr to rgb) but large impact for rgb (with slow
conversion from rgb into gbr)?

>>> diff --git a/tests/ref/fate/utvideoenc_rgb_left
>>> b/tests/ref/fate/utvideoenc_rgb_left
>>> index a1d200096a..1ee7c58564 100644
>>> --- a/tests/ref/fate/utvideoenc_rgb_left
>>> +++ b/tests/ref/fate/utvideoenc_rgb_left
>>
>> Why do they change?
>> Do I understand correctly that the files get bigger (~5%)?
>> If yes, wouldn't that indicate that the patch is not a good idea?
>
> Its because of different scaling path. Have nothing to do with
> good or bad idea.

Thank you, imo this indicates the utvideo rgb tests
should be fixed to use rgb input.

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Paul B Mahol 
On 12/31/17, Carl Eugen Hoyos  wrote:
> 2017-12-31 10:48 GMT+01:00 Paul B Mahol :
>> Signed-off-by: Paul B Mahol 
>> ---
>>  libavcodec/utvideoenc.c   |  47 +---
>>  tests/ref/fate/utvideoenc_rgb_left| 100
>> +-
>>  tests/ref/fate/utvideoenc_rgb_median  | 100
>> +-
>>  tests/ref/fate/utvideoenc_rgb_none| 100
>> +-
>>  tests/ref/fate/utvideoenc_rgba_left   | 100
>> +-
>>  tests/ref/fate/utvideoenc_rgba_median | 100
>> +-
>>  tests/ref/fate/utvideoenc_rgba_none   | 100
>> +-
>>  7 files changed, 327 insertions(+), 320 deletions(-)
>
> Is there a speed impact?
> (Or actually: How much faster is gbr encoding, how much slower rgb
> encoding?)

Very very fast, very very slow.

>
>> diff --git a/tests/ref/fate/utvideoenc_rgb_left
>> b/tests/ref/fate/utvideoenc_rgb_left
>> index a1d200096a..1ee7c58564 100644
>> --- a/tests/ref/fate/utvideoenc_rgb_left
>> +++ b/tests/ref/fate/utvideoenc_rgb_left
>
> Why do they change?
> Do I understand correctly that the files get bigger (~5%)?
> If yes, wouldn't that indicate that the patch is not a good idea?

Its because of different scaling path. Have nothing to do with good or bad idea.

>
> Please add a micro version bump, Carl Eugen

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

Re: [FFmpeg-devel] [PATCH] avcodec/utvideoenc: switch to planar RGB formats

2017-12-31 Thread Carl Eugen Hoyos 
2017-12-31 10:48 GMT+01:00 Paul B Mahol :
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/utvideoenc.c   |  47 +---
>  tests/ref/fate/utvideoenc_rgb_left| 100 
> +-
>  tests/ref/fate/utvideoenc_rgb_median  | 100 
> +-
>  tests/ref/fate/utvideoenc_rgb_none| 100 
> +-
>  tests/ref/fate/utvideoenc_rgba_left   | 100 
> +-
>  tests/ref/fate/utvideoenc_rgba_median | 100 
> +-
>  tests/ref/fate/utvideoenc_rgba_none   | 100 
> +-
>  7 files changed, 327 insertions(+), 320 deletions(-)

Is there a speed impact?
(Or actually: How much faster is gbr encoding, how much slower rgb encoding?)

> diff --git a/tests/ref/fate/utvideoenc_rgb_left 
> b/tests/ref/fate/utvideoenc_rgb_left
> index a1d200096a..1ee7c58564 100644
> --- a/tests/ref/fate/utvideoenc_rgb_left
> +++ b/tests/ref/fate/utvideoenc_rgb_left

Why do they change?
Do I understand correctly that the files get bigger (~5%)?
If yes, wouldn't that indicate that the patch is not a good idea?

Please add a micro version bump, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel