RE: [PATCH 03/17] [media] s5p-mfc: set B-frames as 2 while encoding

[Kamil Debski <k.debski@samsung.com>]

Hi Kiran,

> From: Kiran Avnd [mailto:kiran@chromium.org]
> Sent: Tuesday, September 16, 2014 10:10 AM
> 
> Hi Kamil,
> 
> 
> On Mon, Sep 15, 2014 at 8:12 PM, Kamil Debski <k.debski@samsung.com>
> wrote:
> 
> 
> 	Hi,
> 
> 	> From: Kiran AVND [mailto:avnd.kiran@samsung.com]
> 	> Sent: Monday, September 15, 2014 8:43 AM
> 	>
> 	> set default number of B-frames as 2 while encoding for better
> 	> compression. This User can however change this setting using
> 	> V4L2_CID_MPEG_VIDEO_B_FRAMES.
> 
> 	The last sentence should be rephrased, as it is not clear.
> 
> 
> 
> Sure. Will do.
> 
> 
> 	The tougher question is what should be the default?
> 	In my opinion the default should produce a stream that is as
> 	simple as it gets. It should be playable on most hardware,
> 	some of which may not support B frames.
> 
> 	Anyway - this setting can be changed by the user to 2 using
> 	V4L2_CID_MPEG_VIDEO_B_FRAMES ;)
> 
> 
> 
> 
> Its empirical, and this setting has better compression quality and bit
> rate than otherwise, in our testing.

It doesn't surprise me - introducing B frames should increase both quality
and compression efficiency at the cost of computation complexity and
limiting compatibility.

The thing I am against is changing the driver so that it forces such
value of the encoding parameter on the user. Default number of B frames
should remain 0. The application should consciously choose to use B frames.

Best wishes,
--
Kamil Debski
Samsung R&D Institute Poland


> 
> Regards,
> Kiran
> 
> 
> 
> 
> 	>
> 	> Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
> 	> ---
> 	>  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c |    2 +-
> 	>  1 files changed, 1 insertions(+), 1 deletions(-)
> 	>
> 	> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> 	> b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> 	> index cd1b2a2..f7a6f68 100644
> 	> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> 	> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
> 	> @@ -280,7 +280,7 @@ static struct mfc_control controls[] = {
> 	>               .minimum = 0,
> 	>               .maximum = 2,
> 	>               .step = 1,
> 	> -             .default_value = 0,
> 	> +             .default_value = 2,
> 	>       },
> 	>       {
> 	>               .id = V4L2_CID_MPEG_VIDEO_H264_PROFILE,
> 	> --
> 	> 1.7.3.rc2
> 
> 	--
> 	To unsubscribe from this list: send the line "unsubscribe linux-
> media" in
> 	the body of a message to majordomo@vger.kernel.org
> 	More majordomo info at  http://vger.kernel.org/majordomo-
> info.html
> 
>