raspberrypi / firmware

This repository contains pre-compiled binaries of the current Raspberry Pi kernel and modules, userspace libraries, and bootloader/GPU firmware.
5.17k stars 1.68k forks source link

MMAL freezes on flushing (maybe) #457

Open ghost opened 9 years ago

ghost commented 9 years ago

I observe the following situation:

(Sorry, I don't remember which port hangs when being disabled; if it's important I can look again.)

Is this a MMAL bug, or does it just not allow flushing the decoder if no real input has been sent to it? It also could be a bug in my code.

popcornmix commented 9 years ago

Ping @6by9 to confirm if this is a valid thing to do. A trivial program that demonstrates the problem would be useful for debugging.

6by9 commented 9 years ago

Are we talking image_decode or video_decode here?

Technically sending an EOS is not a flush. EOS should be passed through in sequence to indicate that the stream is complete and no further data should be expected on the port. If you want to flush all buffers from the component back to the buffer supplier you should use mmal_port_flush.

In either image or video case, the input is going to be a compressed format, so the codec is wanting a complete frame to process. I'm guessing that it is stashing all buffers on a queue waiting for the first frame to be decodable and processing in order. As we have 0 bytes of data, it doesn't pass it to the codec as it is insufficient. On port disable, it should automatically flush the port and retrieve all buffers, but I'm guessing your EOS buffer goes astray somehow.

If you can provide a simple test case then one of us can run it with the GPU debugger to see what is going on that side.

ghost commented 9 years ago

Are we talking image_decode or video_decode here?

Video (the h264 decoder).

If you want to flush all buffers from the component back to the buffer supplier you should use mmal_port_flush.

Well yes... in FFmpeg, we call these EOS things flush packets, because they force the decoder to return the remaining queued decoded frames.

If you can provide a simple test case then one of us can run it with the GPU debugger to see what is going on that side.

I can try later.

6by9 commented 9 years ago

If you want to flush all buffers from the component back to the buffer supplier you should use mmal_port_flush.

Well yes... in FFmpeg, we call these EOS things flush packets, because they force the decoder to return the remaining queued decoded frames.

In neither IL nor MMAL does an EOS force a flush. It is an in-band signal that is used on to designate that the end has been reached, and on IL it triggers a callback to the client when received by a sink component. This allows the efficient use of FIFOs within the pipe, and only when they are all emptied will you get to the EOS buffer and the client knows everything is done. It does however require that you continue to service output port buffers until the EOS appears on the output port. Queuing an EOS at the end of your stream and thinking the job is complete is incorrect.

It does sound like a bug in the video_decode component, but a test case would confirm. Are you passing in header bytes first via OMX_IndexParamCodecConfig, or passing those in-band with the rest of the stream? It changes the behaviour subtly within the component.

ghost commented 9 years ago

It does however require that you continue to service output port buffers until the EOS appears on the output port. Queuing an EOS at the end of your stream and thinking the job is complete is incorrect.

So my thinking was:

  1. write EOS to the input port
  2. keep reading buffers from the output port
  3. if a buffer from the output port is marked EOS, consider everything done

Is this correct?

Are you passing in header bytes first via OMX_IndexParamCodecConfig, or passing those in-band with the rest of the stream? It changes the behaviour subtly within the component.

I'm not sure what OMX_IndexParamCodecConfig exactly is, and what it encompasses. The FFmpeg wrapper converts all packets to Annex B format. So there are no header bytes; they should be coming inline with the packet data, whose boundaries are marked with MMAL_BUFFER_HEADER_FLAG_FRAME_START and ..._END. (But don't ask me about any subtleties - the FFmpeg code for this, h264_mp4toannexb_bsf.c, is very fragile and just does about its job.)

The initialization code is here: http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/mmaldec.c#l259 But of course this is complex. I'll try to make a test case to reproduce the exact situation I'm running into.

6by9 commented 9 years ago

Is this correct?

Yes, correct. Glad we both have the same understanding, even if our language wasn't always clear :-)

I'm not sure what OMX_IndexParamCodecConfig exactly is, and what it encompasses.

It's a way of passing in header bytes out of band as that is easier with some containers (so I'm told - not something I have too much experience of). I seem to recall discussions with popcornmix over it and XBMC/Kodi at one point. All data via buffers is fine - I just wanted to remove one extra variable when trying to debug.

I was just thinking I might be able to mod the hello_video app easily for test purposes, but actually https://github.com/raspberrypi/userland/blob/master/host_applications/android/apps/vidtex/svp.c looks a better candidate with a partial knobble of the container reader. It's unlikely to be today for me though (slim chance of this evening).

What MMAL encoding are your output buffers? Opaque, I420, or other? Again just narrowing down the number of variables I need to check.

ghost commented 9 years ago

Yes, correct. Glad we both have the same understanding, even if our language wasn't always clear :-)

Too many terms for the same thing!

It's a way of passing in header bytes out of band as that is easier with some containers (so I'm told - not something I have too much experience of). I seem to recall discussions with popcornmix over it and XBMC/Kodi at one point.

(Being able to pass in mp4/mkv style data directly would be nice too. I'm not sure if that's what can be done with this API.)

What MMAL encoding are your output buffers? Opaque, I420, or other? Again just narrowing down the number of variables I need to check.

MMAL_ENCODING_OPAQUE.

I was just thinking I might be able to mod the hello_video app easily for test purposes, but actually https://github.com/raspberrypi/userland/blob/master/host_applications/android/apps/vidtex/svp.c looks a better candidate with a partial knobble of the container reader.

Looked a bit too complicated. But I managed to strip down the ffmpeg decoder wrapper to a relatively small program that predictably freezes:

http://sprunge.us/RLUD

// Compile as:
// gcc mmaltest.c -isystem/opt/vc/include/ -isystem/opt/vc/include/interface/vcos/pthreads -isystem/opt/vc/include/interface/vmcs_host/linux -fgnu89-inline -L/opt/vc/lib -lmmal_core -lmmal_util -lmmal_vc_client -lbcm_host -g -o mmaltest

#include <stdio.h>

#include <bcm_host.h>
#include <interface/mmal/mmal.h>
#include <interface/mmal/util/mmal_util.h>
#include <interface/mmal/util/mmal_util_params.h>
#include <interface/mmal/util/mmal_default_components.h>
#include <interface/mmal/vc/mmal_vc_api.h>

MMAL_COMPONENT_T *decoder;
MMAL_QUEUE_T *queue_decoded_frames;
MMAL_POOL_T *pool_in;
MMAL_POOL_T *pool_out;

#define FFMAX(a, b) ((a) < (b) ? (b) : (a))

static void ffmmal_stop_decoder(void)
{
    MMAL_BUFFER_HEADER_T *buffer;

    mmal_port_disable(decoder->input[0]);
    mmal_port_disable(decoder->output[0]);
    mmal_port_disable(decoder->control);

    mmal_port_flush(decoder->input[0]);
    mmal_port_flush(decoder->output[0]);
    mmal_port_flush(decoder->control);

    while ((buffer = mmal_queue_get(queue_decoded_frames)))
        mmal_buffer_header_release(buffer);
}

static void input_callback(MMAL_PORT_T *port, MMAL_BUFFER_HEADER_T *buffer)
{
    mmal_buffer_header_release(buffer);
}

static void output_callback(MMAL_PORT_T *port, MMAL_BUFFER_HEADER_T *buffer)
{
    mmal_queue_put(queue_decoded_frames, buffer);
}

static void control_port_cb(MMAL_PORT_T *port, MMAL_BUFFER_HEADER_T *buffer)
{
    MMAL_STATUS_T status;

    if (buffer->cmd == MMAL_EVENT_ERROR) {
        status = *(uint32_t *)buffer->data;
        printf("MMAL error %d on control port\n", (int)status);
    } else {
        printf("Unknown MMAL event on control port\n");
    }

    mmal_buffer_header_release(buffer);
}

static void ffmal_update_format(void)
{
    MMAL_STATUS_T status;
    MMAL_ES_FORMAT_T *format_out = decoder->output[0]->format;

    assert(format_out);

    if ((status = mmal_port_parameter_set_uint32(decoder->output[0], MMAL_PARAMETER_EXTRA_BUFFERS, 10)))
        assert(0);

    format_out->encoding = MMAL_ENCODING_OPAQUE;

    if ((status = mmal_port_format_commit(decoder->output[0])))
        assert(0);

    decoder->output[0]->buffer_size =
        FFMAX(decoder->output[0]->buffer_size_min, decoder->output[0]->buffer_size_recommended);
    decoder->output[0]->buffer_num =
        FFMAX(decoder->output[0]->buffer_num_min, decoder->output[0]->buffer_num_recommended) + 10;
    pool_out = mmal_pool_create(decoder->output[0]->buffer_num,
                                      decoder->output[0]->buffer_size);
    assert(pool_out);
}

static void ffmmal_flush(void)
{
    MMAL_STATUS_T status;

    printf("flush\n");

    ffmmal_stop_decoder();

    if ((status = mmal_port_enable(decoder->control, control_port_cb)))
        assert(0);
    if ((status = mmal_port_enable(decoder->input[0], input_callback)))
        assert(0);
    if ((status = mmal_port_enable(decoder->output[0], output_callback)))
        assert(0);

    printf("end flush\n");
}

int main(int argc, char **argv)
{
    MMAL_STATUS_T status;
    MMAL_ES_FORMAT_T *format_in;

    bcm_host_init();
    mmal_vc_init();

    if ((status = mmal_component_create(MMAL_COMPONENT_DEFAULT_VIDEO_DECODER, &decoder)))
        assert(0);

    format_in = decoder->input[0]->format;
    format_in->type = MMAL_ES_TYPE_VIDEO;
    format_in->encoding = MMAL_ENCODING_H264;
    format_in->es->video.width = 1280;
    format_in->es->video.height = 720;
    format_in->es->video.crop.width = 1280;
    format_in->es->video.crop.height = 720;
    format_in->es->video.frame_rate.num = 24000;
    format_in->es->video.frame_rate.den = 1001;
    format_in->es->video.par.num = 1;
    format_in->es->video.par.den = 1;
    format_in->flags = MMAL_ES_FORMAT_FLAG_FRAMED;

    if ((status = mmal_port_format_commit(decoder->input[0])))
        assert(0);

    decoder->input[0]->buffer_num =
        FFMAX(decoder->input[0]->buffer_num_min, 20);
    decoder->input[0]->buffer_size =
        FFMAX(decoder->input[0]->buffer_size_min, 512 * 1024);
    pool_in = mmal_pool_create(decoder->input[0]->buffer_num, 0);
    assert(pool_in);

    ffmal_update_format();

    queue_decoded_frames = mmal_queue_create();
    assert(queue_decoded_frames);

    decoder->input[0]->userdata = (void*)"";
    decoder->output[0]->userdata = (void*)"";
    decoder->control->userdata = (void*)"";

    if ((status = mmal_port_enable(decoder->control, control_port_cb)))
        assert(0);
    if ((status = mmal_port_enable(decoder->input[0], input_callback)))
        assert(0);
    if ((status = mmal_port_enable(decoder->output[0], output_callback)))
        assert(0);

    if ((status = mmal_component_enable(decoder)))
        assert(0);

    MMAL_BUFFER_HEADER_T *mbuffer;

    mbuffer = mmal_queue_get(pool_in->queue);
    if (!mbuffer)
        assert(0);

    mmal_buffer_header_reset(mbuffer);
    mbuffer->cmd = 0;
    mbuffer->pts = 0;
    mbuffer->dts = 0;
    mbuffer->flags = MMAL_BUFFER_HEADER_FLAG_EOS | MMAL_BUFFER_HEADER_FLAG_FRAME_END | MMAL_BUFFER_HEADER_FLAG_FRAME_START;
    mbuffer->data = "";
    mbuffer->length = 0;
    mbuffer->user_data = NULL;
    mbuffer->alloc_size = decoder->input[0]->buffer_size;

    if ((status = mmal_port_send_buffer(decoder->input[0], mbuffer)))
        assert(0);

    ffmmal_flush();

    printf("done\n");
    return 0;
}
6by9 commented 9 years ago

I've finally had a chance to run your test - thank you very much for providing it.

Bizarre one - on flush the buffer appears to be submitted for a VCHI (bulk?) transfer even though it is empty, and that doesn't complete. If you ctrl-c, the transfer gets aborted and that releases the buffer correctly. I'd need more time to investigate why the transfer gets stalled, and why that doesn't happen normally (empty buffers with flags aren't abnormal).

So it looks like a bug in the MMAL framework code - the video_decode component is doing the right thing (other than not forwarding on the EOS immediately).

ghost commented 9 years ago

Thanks for having a look!

JamesH65 commented 6 years ago

@6by9 Did anything ever come of this?

6by9 commented 6 years ago

Not that I recall.

JamesH65 commented 6 years ago

I'll leave open.

JamesH65 commented 6 years ago

@6by9 Anyhing to report/ Should we simply 30 day this one?

PredatorMF commented 3 years ago

Is there any solution to this ?

I get the same exact problem. After sending MMAL_BUFFER_HEADER_FLAG_EOS, the decoder stops outputting any frames. Trying to close the decoder after EOS is sent hangs in the mmal_port_disable call.

AMCerasoli commented 2 years ago

This could be why Omxplayer sometimes hangs after looping a video for hours? Sometimes you just press a key and continue playing...

cyph84 commented 2 years ago

I think I encountered a variation of this bug in ffmpeg. It seems like calling disable on the decoder port without sending any data causes mmal to freeze.

Here is my patch for ffmpeg:

https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210916134652.22781-1-cyph1984@gmail.com/

Here is another patch that tries to workaround the same problem in mpv:

https://github.com/mpv-player/mpv/pull/9189