Re: [FFmpeg-devel] [PATCH 07/14] mediacodec: check whether cropping is set before use

[Matthieu Bouron]

On Sat, Dec 16, 2017 at 07:20:53PM +0800, Wang Bin wrote:
> 2017-12-16 17:12 GMT+08:00 Matthieu Bouron :
> > On Sat, Dec 16, 2017 at 01:40:18PM +0800, Wang Bin wrote:
> >> > > +width = s->crop_right + 1 - s->crop_left;
> >> > > +else
> >> > > +width = s->width;
> >> > >
> >> > >  av_log(avctx, AV_LOG_INFO,
> >> > >  "Output crop parameters top=%d bottom=%d left=%d right=%d, "
> >> >
> >> > On which device does this happen ?
> >>
> >>
> >> None of my devices have such problem. It happens if replace jni by ndk
> >> mediacodec functions(maybe another patch later). original code:
> >> https://github.com/aosp-mirror/platform_frameworks_base/blob/master/media/java/android/media/MediaCodec.java#L190
> >
> > OK. I will soon apply the patch.
> >
> > I'm however not in favor of replacing the MediaCodec jni code by its ndk
> > counterpart now as it would drop compatibility with Android 4.4.
> 
> Just load libmediandk.so and resolve symbols at runtime, and fallback
> to jni if the library does not exist, i.e. on android < 5.0

I don't really want to maintain both code paths as it will add complexity.
If we are to use the ndk I would also prefer to link against the library
directly instead of loading its symbols at runtime.

Note: I'm not against switching to the NDK in the future as it would allow
us to remove the jni dependency, but it's a bit too early IMHO.

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

Re: [FFmpeg-devel] [PATCH 07/14] mediacodec: check whether cropping is set before use

[Wang Bin]

2017-12-16 17:12 GMT+08:00 Matthieu Bouron :
> On Sat, Dec 16, 2017 at 01:40:18PM +0800, Wang Bin wrote:
>> > > +width = s->crop_right + 1 - s->crop_left;
>> > > +else
>> > > +width = s->width;
>> > >
>> > >  av_log(avctx, AV_LOG_INFO,
>> > >  "Output crop parameters top=%d bottom=%d left=%d right=%d, "
>> >
>> > On which device does this happen ?
>>
>>
>> None of my devices have such problem. It happens if replace jni by ndk
>> mediacodec functions(maybe another patch later). original code:
>> https://github.com/aosp-mirror/platform_frameworks_base/blob/master/media/java/android/media/MediaCodec.java#L190
>
> OK. I will soon apply the patch.
>
> I'm however not in favor of replacing the MediaCodec jni code by its ndk
> counterpart now as it would drop compatibility with Android 4.4.

Just load libmediandk.so and resolve symbols at runtime, and fallback
to jni if the library does not exist, i.e. on android < 5.0
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 07/14] mediacodec: check whether cropping is set before use

[Matthieu Bouron]

On Sat, Dec 16, 2017 at 01:40:18PM +0800, Wang Bin wrote:
> > > +width = s->crop_right + 1 - s->crop_left;
> > > +else
> > > +width = s->width;
> > >
> > >  av_log(avctx, AV_LOG_INFO,
> > >  "Output crop parameters top=%d bottom=%d left=%d right=%d, "
> >
> > On which device does this happen ?
> 
> 
> None of my devices have such problem. It happens if replace jni by ndk
> mediacodec functions(maybe another patch later). original code:
> https://github.com/aosp-mirror/platform_frameworks_base/blob/master/media/java/android/media/MediaCodec.java#L190

OK. I will soon apply the patch.

I'm however not in favor of replacing the MediaCodec jni code by its ndk
counterpart now as it would drop compatibility with Android 4.4.

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

Re: [FFmpeg-devel] [PATCH 07/14] mediacodec: check whether cropping is set before use

[Wang Bin]

> > +width = s->crop_right + 1 - s->crop_left;
> > +else
> > +width = s->width;
> >
> >  av_log(avctx, AV_LOG_INFO,
> >  "Output crop parameters top=%d bottom=%d left=%d right=%d, "
>
> On which device does this happen ?


None of my devices have such problem. It happens if replace jni by ndk
mediacodec functions(maybe another patch later). original code:
https://github.com/aosp-mirror/platform_frameworks_base/blob/master/media/java/android/media/MediaCodec.java#L190
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 07/14] mediacodec: check whether cropping is set before use

[Matthieu Bouron]

On Fri, Dec 15, 2017 at 03:06:49PM +0800, wbse...@gmail.com wrote:
> From: wang-bin 

Hi,

> 
> ---
>  libavcodec/mediacodecdec_common.c | 23 +--
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/libavcodec/mediacodecdec_common.c 
> b/libavcodec/mediacodecdec_common.c
> index cb2f6ae5e5..05d3bcd4b5 100644
> --- a/libavcodec/mediacodecdec_common.c
> +++ b/libavcodec/mediacodecdec_common.c
> @@ -412,20 +412,15 @@ static int mediacodec_dec_parse_format(AVCodecContext 
> *avctx, MediaCodecDecConte
>  }
>  
>  /* Optional fields */
> -if (ff_AMediaFormat_getInt32(s->format, "crop-top", ))
> -s->crop_top = value;
> -
> -if (ff_AMediaFormat_getInt32(s->format, "crop-bottom", ))
> -s->crop_bottom = value;
> -
> -if (ff_AMediaFormat_getInt32(s->format, "crop-left", ))
> -s->crop_left = value;
> -
> -if (ff_AMediaFormat_getInt32(s->format, "crop-right", ))
> -s->crop_right = value;
> -
> -width = s->crop_right + 1 - s->crop_left;
> -height = s->crop_bottom + 1 - s->crop_top;
> +if (ff_AMediaFormat_getInt32(s->format, "crop-top", >crop_top) && 
> ff_AMediaFormat_getInt32(s->format, "crop-bottom", >crop_bottom))

Please break this line after &&.

> +height = s->crop_bottom + 1 - s->crop_top;
> +else
> +height = s->height;
> +
> +if (ff_AMediaFormat_getInt32(s->format, "crop-left", >crop_left) && 
> ff_AMediaFormat_getInt32(s->format, "crop-right", >crop_right))

Same here.

> +width = s->crop_right + 1 - s->crop_left;
> +else
> +width = s->width;
>  
>  av_log(avctx, AV_LOG_INFO,
>  "Output crop parameters top=%d bottom=%d left=%d right=%d, "

On which device does this happen ?

Thanks,

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

[FFmpeg-devel] [PATCH 07/14] mediacodec: check whether cropping is set before use

[wbsecg1]

From: wang-bin 

---
 libavcodec/mediacodecdec_common.c | 23 +--
 1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/libavcodec/mediacodecdec_common.c 
b/libavcodec/mediacodecdec_common.c
index cb2f6ae5e5..05d3bcd4b5 100644
--- a/libavcodec/mediacodecdec_common.c
+++ b/libavcodec/mediacodecdec_common.c
@@ -412,20 +412,15 @@ static int mediacodec_dec_parse_format(AVCodecContext 
*avctx, MediaCodecDecConte
 }
 
 /* Optional fields */
-if (ff_AMediaFormat_getInt32(s->format, "crop-top", ))
-s->crop_top = value;
-
-if (ff_AMediaFormat_getInt32(s->format, "crop-bottom", ))
-s->crop_bottom = value;
-
-if (ff_AMediaFormat_getInt32(s->format, "crop-left", ))
-s->crop_left = value;
-
-if (ff_AMediaFormat_getInt32(s->format, "crop-right", ))
-s->crop_right = value;
-
-width = s->crop_right + 1 - s->crop_left;
-height = s->crop_bottom + 1 - s->crop_top;
+if (ff_AMediaFormat_getInt32(s->format, "crop-top", >crop_top) && 
ff_AMediaFormat_getInt32(s->format, "crop-bottom", >crop_bottom))
+height = s->crop_bottom + 1 - s->crop_top;
+else
+height = s->height;
+
+if (ff_AMediaFormat_getInt32(s->format, "crop-left", >crop_left) && 
ff_AMediaFormat_getInt32(s->format, "crop-right", >crop_right))
+width = s->crop_right + 1 - s->crop_left;
+else
+width = s->width;
 
 av_log(avctx, AV_LOG_INFO,
 "Output crop parameters top=%d bottom=%d left=%d right=%d, "
-- 
2.15.1

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