nyanmisaka / ffmpeg-rockchip

FFmpeg with async and zero-copy Rockchip MPP & RGA support
Other
426 stars 57 forks source link

Adding MJPEG #83

Closed benhoff closed 1 month ago

benhoff commented 2 months ago

Hey I dug through the rkmppdec.c to see where MJPEG might need additional handling.

I specifically looked at areas where avctx->codec_id is handled.

I don't know that AFBC needs to be handled for MJEPG.

I don't believe that there is such a thing as sample aspect ratio.

I don't think there's an issue with mastering display for MJPEG.

FFMPEG and container formats is not my jam however. Is there something I'm missing that needs to be added or addressed? Happy to take a swing at this.

nyanmisaka commented 2 months ago

This won't work, you can try it out.

Unlike other codecs, MJPEG decoding in MPP uses a special advanced interface, so it can't reuse existing logic.

benhoff commented 2 months ago

Would it make sense to port this code into the existing code base?

https://github-com.translate.goog/rockchip-linux/mpp/issues/592?_x_tr_sl=zh-CN&_x_tr_tl=en&_x_tr_hl=zh-CN&_x_tr_pto=wapp#issuecomment-2101716446

nyanmisaka commented 2 months ago

This makes sense, but in practice it would be better to unify the interfaces in MPP.

benhoff commented 2 months ago

Is it the idea that the simple api is using somewhat syncronous interfaces and the advanced is using more of an epoll?

So is the idea that we need to flip to more of an epoll based code similar to this block of code? https://github.com/rockchip-linux/mpp/blob/239e15eaebf98f68c0093b00ff8108d23b0147e7/test/mpi_dec_test.c#L336

Rather than using the function decode_get_frame as it's currently leveraged in the ffmpeg fork?

Is the thought that once you've made the change to the advanced interface you would use that as the mainline for all code?

Sorry to prod on this, I'm trying to get something out to market and did not expect this much complexity, so I'm trying to figure out if I can patch out of this or if I should figure out a workaround.

Note to myself: probably need to follow this line of code to change output format before JPEG decoding: mpi_dec_test.c

nyanmisaka commented 2 months ago

Is it the idea that the simple api is using somewhat syncronous interfaces and the advanced is using more of an epoll?

https://github.com/rockchip-linux/mpp/blob/239e15eaebf98f68c0093b00ff8108d23b0147e7/inc/rk_mpi.h#L50-L52

So is the idea that we need to flip to more of an epoll based code similar to this block of code?

Nope. According to RK developers, the simple interface is recommended anyway.

But the MJPEG decode is an exception, they have not yet implemented the simple interface for it (the simple interface calls MppTask/advanced interface internally).

benhoff commented 1 month ago

I ended up purchasing a USB3.0 capture card that didn't need mjpeg decoding.

I sat down several times to try and commit something, but the complexity of the existing implementation of ffmpeg-rockchip and available examples of mpp outstrip my skills to try and pull something clean together.