Re: [FFmpeg-devel] [PATCH 2/6] vaapi: Remove H.264 baseline profile

2017-10-09 Thread Carl Eugen Hoyos 
2017-10-09 13:21 GMT+02:00 wm4 :

> Shouldn't trying to decode baseline video just fall back to sw decoding?

Given that software doesn't support the specific features of baseline either,
I don't think this helps.
I was under the impression the only sane thing to do when reading
baseline H.264 is printing a warning and continue as if constraint
baseline was detected.

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

Re: [FFmpeg-devel] [PATCH 2/6] vaapi: Remove H.264 baseline profile

2017-10-09 Thread wm4 
On Sun, 8 Oct 2017 16:49:58 +0100
Mark Thompson  wrote:

> This has been deprecated in libva2 because hardware does not and will not
> support it.  Therefore never consider it for decode, and for encode assume
> the user meant constrained baseline profile instead.
> ---
> On 08/10/17 16:44, Derek Buitenhuis wrote:
> > On 10/8/2017 4:11 PM, Mark Thompson wrote:  
> >> +case FF_PROFILE_H264_BASELINE:
> >> +// Baseline profile is not supported, assume the user meant
> >> +// constrained baseline instead.
> >> +avctx->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;  
> > 
> > Trying to automatically (and silently!) guess what the user wanted
> > is never a good idea, IMO. At the very least, print a warning.  
> 
> Yeah, ok, I agree.  Patch changed as enclosing.
> 
> 
>  libavcodec/vaapi_decode.c  |  1 -
>  libavcodec/vaapi_encode_h264.c | 12 
>  2 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/libavcodec/vaapi_decode.c b/libavcodec/vaapi_decode.c
> index cf58aae4c6..4f0ff84e01 100644
> --- a/libavcodec/vaapi_decode.c
> +++ b/libavcodec/vaapi_decode.c
> @@ -246,7 +246,6 @@ static const struct {
>  MAP(MPEG4,   MPEG4_MAIN,  MPEG4Main   ),
>  MAP(H264,H264_CONSTRAINED_BASELINE,
> H264ConstrainedBaseline),
> -MAP(H264,H264_BASELINE,   H264Baseline),
>  MAP(H264,H264_MAIN,   H264Main),
>  MAP(H264,H264_HIGH,   H264High),
>  #if VA_CHECK_VERSION(0, 37, 0)
> diff --git a/libavcodec/vaapi_encode_h264.c b/libavcodec/vaapi_encode_h264.c
> index 549867ef3f..efde80b08e 100644
> --- a/libavcodec/vaapi_encode_h264.c
> +++ b/libavcodec/vaapi_encode_h264.c
> @@ -1175,6 +1175,10 @@ static av_cold int 
> vaapi_encode_h264_init(AVCodecContext *avctx)
>  ctx->codec = _encode_type_h264;
>  
>  switch (avctx->profile) {
> +case FF_PROFILE_H264_BASELINE:
> +av_log(avctx, AV_LOG_WARNING, "H.264 baseline profile is not "
> +   "supported, using constrained baseline profile instead.\n");
> +avctx->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
>  case FF_PROFILE_H264_CONSTRAINED_BASELINE:
>  ctx->va_profile = VAProfileH264ConstrainedBaseline;
>  if (avctx->max_b_frames != 0) {
> @@ -1183,14 +1187,6 @@ static av_cold int 
> vaapi_encode_h264_init(AVCodecContext *avctx)
> "doesn't support encoding with B frames, disabling 
> them.\n");
>  }
>  break;
> -case FF_PROFILE_H264_BASELINE:
> -ctx->va_profile = VAProfileH264Baseline;
> -if (avctx->max_b_frames != 0) {
> -avctx->max_b_frames = 0;
> -av_log(avctx, AV_LOG_WARNING, "H.264 baseline profile "
> -   "doesn't support encoding with B frames, disabling 
> them.\n");
> -}
> -break;
>  case FF_PROFILE_H264_MAIN:
>  ctx->va_profile = VAProfileH264Main;
>  break;

Shouldn't trying to decode baseline video just fall back to sw decoding?
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/6] vaapi: Remove H.264 baseline profile

2017-10-09 Thread Moritz Barsnick 
On Sun, Oct 08, 2017 at 16:49:58 +0100, Mark Thompson wrote:
>  switch (avctx->profile) {
> +case FF_PROFILE_H264_BASELINE:
> +av_log(avctx, AV_LOG_WARNING, "H.264 baseline profile is not "
> +   "supported, using constrained baseline profile instead.\n");
> +avctx->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;
>  case FF_PROFILE_H264_CONSTRAINED_BASELINE:

I recall a discussion that linting/analysis tools such as Coverty
require a fall-through to be marked as such.

> /* fall-through */
> case FF_PROFILE_H264_CONSTRAINED_BASELINE:

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

Re: [FFmpeg-devel] [PATCH 2/6] vaapi: Remove H.264 baseline profile

2017-10-08 Thread Derek Buitenhuis 
On 10/8/2017 4:49 PM, Mark Thompson wrote:
> Yeah, ok, I agree.  Patch changed as enclosing.
> 
> 
>  libavcodec/vaapi_decode.c  |  1 -
>  libavcodec/vaapi_encode_h264.c | 12 
>  2 files changed, 4 insertions(+), 9 deletions(-)

Looks OK to me. I assume we don't care about the old lib
versions and hardware wanting to use the profile.

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

Re: [FFmpeg-devel] [PATCH 2/6] vaapi: Remove H.264 baseline profile

2017-10-08 Thread Derek Buitenhuis 
On 10/8/2017 4:11 PM, Mark Thompson wrote:
> +case FF_PROFILE_H264_BASELINE:
> +// Baseline profile is not supported, assume the user meant
> +// constrained baseline instead.
> +avctx->profile = FF_PROFILE_H264_CONSTRAINED_BASELINE;

Trying to automatically (and silently!) guess what the user wanted
is never a good idea, IMO. At the very least, print a warning.

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