[mythtv] Seg Fault in mediacodec

Thu Jul 5 21:16:43 UTC 2018

On 07/05/2018 04:58 PM, Peter Bennett wrote:
>
>
>
> On 07/05/2018 04:49 PM, Aman Gupta wrote:
>>
>> On Thu, Jul 5, 2018 at 4:04 PM Peter Bennett <pb.mythtv at gmail.com 
>> <mailto:pb.mythtv at gmail.com>> wrote:
>>
>>
>>
>>     On 07/05/2018 01:08 PM, David Engel wrote:
>>>>     I reduced the number of buffers so that it displays a couple of frames
>>>>     before crashing. I see a fat green line along the bottom. I suspect that it
>>>>     is trying to use a frame size of 1920x1088 instead of 1920x1080 (see the
>>>>     trace I sent, it shows slice-height 1088). I plan to step through the ffmpeg
>>>>     code and see if perhaps it is copying 8 rows more than it should. I don't
>>>>     know what the slice-height refers to, but it looks suspicious.
>>>>
>>>>     It may be that the problem is associated with 1080 rows. Of the resolutions
>>>>     I have tried, only 1080 is not divisible by 16. I have tried 480, 720, 2160
>>>>     - all work without seg fault.
>>>     You could be on to something.  The a/v side of things is not an area
>>>     with which I'm intimately familier, however, I vaguely recall various
>>>     issues with 1080 vs. 1088 popping up in the past.  Where's Mike Dean
>>>     when we need him?:)
>>     It turns out that is in fact the cause of the crash. The second
>>     plane in the buffer follows on immediately after 1080 lines of
>>     the first plane, but the code assumes it is after 1088 lines of
>>     the first plane, based on slice_height. Is slice_height supposed
>>     to tell the height of the plane? It crashes on copying data for
>>     the second plane when it hits the actual end of the data and
>>     starts accessing addresses beyond that.
>>
>>     This is the patch that fixes it. Instead of using slice_height
>>     here, use height. I suspect that this may cause a failure in
>>     Nougat or other systems if they do in fact have a gap between planes.
>>
>>     --- a/mythtv/external/FFmpeg/libavcodec/mediacodec_sw_buffer.c
>>     +++ b/mythtv/external/FFmpeg/libavcodec/mediacodec_sw_buffer.c
>>     @@ -150,7 +150,8 @@ void
>>     ff_mediacodec_sw_buffer_copy_yuv420_semi_planar(AVCodecContext
>>     *avctx,
>>              } else if (i == 1) {
>>                  height = avctx->height / 2;
>>
>>     -            src += s->slice_height * s->stride;
>>     +            src += s->height * s->stride;
>>                  src += s->crop_top * s->stride;
>>                  src += s->crop_left;
>>              }
>>
>>     Aman - please let me know if this is the correct fix and if you
>>     can incorporate it into FFmpeg.
>>
>>
>> I'm not very familiar with the s/w decoding code paths. I believe 
>> this part of the code came from gstreamer so it might be useful to 
>> see what (if anything) they've done to deal with Oreo.
>>
>> If this breaks Nougat then obviously it cannot be committed as-is.
>>
>> IIRC slice_height is supposed to return the size of the buffer so I'm 
>> pretty sure the correct thing to do here is to continue using 
>> slice_height. It's possible that adding the crop values on top is 
>> what's causing the buffer overflow- maybe those weren't set before Oreo?
>>
>> Aman
>>
>>
>>
>>     Peter
>>
>
> Here is the output from explore on my Oreo system when it got the seg 
> fault. You can see that crop_left is zero so leaving that out will not 
> help. It seems to me that this may be a bug in Oreo or the Shield 
> version of it.
>
>   The value of '*s' is of type 'MediaCodecDecContext' which is a 
> typedef of type 'struct MediaCodecDecContext'
> The value of '*s' is a struct/class of type 'struct 
> MediaCodecDecContext' with the following fields:
>
>                  avctx = <Enter 0 to explore this field of type 
> 'AVCodecContext *'>
>               refcount = 1 .. (Value of type 'atomic_int')
>        hw_buffer_count = 0 .. (Value of type 'atomic_int')
>             codec_name = <Enter 3 to explore this field of type 'char *'>
>                  codec = <Enter 4 to explore this field of type 
> 'FFAMediaCodec *'>
>                 format = <Enter 5 to explore this field of type 
> 'FFAMediaFormat *'>
>                surface = <Enter 6 to explore this field of type 'void *'>
>                started = 0 .. (Value of type 'int')
>               draining = 0 .. (Value of type 'int')
>               flushing = 0 .. (Value of type 'int')
>                    eos = 0 .. (Value of type 'int')
>                  width = 1920 .. (Value of type 'int')
>                 height = 1080 .. (Value of type 'int')
>                 stride = 1920 .. (Value of type 'int')
>           slice_height = 1088 .. (Value of type 'int')
>           color_format = 21 .. (Value of type 'int')
>               crop_top = 0 .. (Value of type 'int')
>            crop_bottom = 1079 .. (Value of type 'int')
>              crop_left = 0 .. (Value of type 'int')
>             crop_right = 1919 .. (Value of type 'int')
>          display_width = 1920 .. (Value of type 'int')
>         display_height = 1080 .. (Value of type 'int')
>    output_buffer_count = <Enter 22 to explore this field of type 
> 'uint64_t'>
>   current_input_buffer = <Enter 23 to explore this field of type 
> 'ssize_t'>
>            delay_flush = false .. (Value of type '_Bool')
>                 serial = 1 .. (Value of type 'atomic_int')
>
>
Aman

You could check the size of the buffer received. This is the trace log 
message -

[mpeg2_mediacodec @ 0x2afc77f378] Got output buffer 7 offset=0 
size=3110400 ts=53009921316 flags=0

Buffer size of 3110400 is equal to 1920*1080 + 1920 * 540.

If you use the slice_height you access 1920 * 1088 + 1920 * 540 = 
3125760 bytes which is more than the buffer size.

Using the buffer size you can tell whether height or slice_height should 
be used.

Peter
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mythtv.org/pipermail/mythtv-dev/attachments/20180705/788b30d0/attachment.html>