JeffyCN / FFmpeg

FFmpeg with rkmpp hwdec - (deprecated due to license issue, check gstreamer instead or https://github.com/nyanmisaka/ffmpeg-rockchip)
https://ffmpeg.org
Other
60 stars 18 forks source link

rkmppdec interlacing detection is incorrect #10

Closed trollcop closed 1 year ago

trollcop commented 1 year ago

https://github.com/JeffyCN/FFmpeg/blob/master/libavcodec/rkmppdec.c#L553

Here, from what I can tell, the frame->interlaced flag is not set correctly. In a standard interlaced source, mppframe->mode would be something like 0x7 (0b111), with the bottom 2 bits specifying that top and bottom frames exist, and 3rd bit specifying that it's in MPP_FRAME_FLAG_TOP_FIRST mode. (Alternatively, it would be 0b1011, specifying MPP_FRAME_FLAG_BOT_FIRST mode).

since MPP_FRAME_FLAG_FIELD_ORDER_MASK define is 0b1100, the

frame->interlaced_frame = ((mode & MPP_FRAME_FLAG_FIELD_ORDER_MASK) == MPP_FRAME_FLAG_DEINTERLACED);

should instead be

frame->interlaced_frame = (mode & MPP_FRAME_FLAG_FIELD_ORDER_MASK) ? 1 : 0;

To be set whether the field order is top or bottom.

How I arrived to this conclusion: I was feeding known interlaced mpeg2ts to ffmpeg with the following comand line:

`ffmpeg -i udp://127.0.0.1:1234?fifo_size=49152 -map 0:p:1064:v -map 0:p:1064:a -acodec copy -vcodec h264_rkmpp -b:v 3M -bufsize 6000K -f mpegts -muxrate 3500K udp://127.0.0.1:5000?pkt_size=1316'

and before the patch it would say the input and output were progressive.

with the patch, the output is following:

Stream #0:0: Video: h264, drm_prime(bt709, top coded first (swapped)), 1440x1080, q=2-31, 3000 kb/s, 29.97 fps, 90k tbn which is more correct.

I am not sure if this has any effect on interlace encoding optimizations for rkmpp_enc.

JeffyCN commented 1 year ago

Hi, this part of code is the same as the original upstream version, so guess you can discuss it in upstream ffmpeg.

from what it know, most of rockchip chips has an iep hw module for mpp deinterlacing, so normally the output frames should be deinterlaced, the (mode & mask) should equal deinterlaced, which means interlaced_frame = 0.

and there's no rkmpp_enc for ffmpeg now.

trollcop commented 1 year ago

and there's no rkmpp_enc for ffmpeg now.

Yeah, i am using mppenc-supporting branch from here: https://github.com/jjm2473/ffmpeg-rk but the decode part in it is same.

from what it know, most of rockchip chips has an iep hw module for mpp deinterlacing, so normally the output frames should be deinterlaced, the (mode & mask) should equal deinterlaced, which means interlaced_frame = 0.

Isn't this logic still wrong? mode & mask would be 0b1100, which would equal the MPP_FRAME_FLAG_DEINTERLACED, thus setting interlaced flag IF both fields are set -> returning deinterlaced from hardware decoder (?)

most of rockchip chips has an iep hw module for mpp deinterlacing

How do I enable this hardware deinterlacing then? I don't see any documentation for that.

JeffyCN commented 1 year ago

and there's no rkmpp_enc for ffmpeg now.

Yeah, i am using mppenc-supporting branch from here: https://github.com/jjm2473/ffmpeg-rk but the decode part in it is same.

hmm, i think the hardest part of mpp enc would be input frame conversion(with hardware accel), that branch uses legacy RGA, which is broken for newer rockchip chips(with kernel 5.10).

from what it know, most of rockchip chips has an iep hw module for mpp deinterlacing, so normally the output frames should be deinterlaced, the (mode & mask) should equal deinterlaced, which means interlaced_frame = 0.

Isn't this logic still wrong? mode & mask would be 0b1100, which would equal the MPP_FRAME_FLAG_DEINTERLACED, thus setting interlaced flag IF both fields are set -> returning deinterlaced from hardware decoder (?)

guess so, maybe discuss it in upstream ffmpeg, might be something like !((mode & mask) == deinterlaced)

most of rockchip chips has an iep hw module for mpp deinterlacing

How do I enable this hardware deinterlacing then? I don't see any documentation for that.

that should be enabled by default (in kernel defconfig + dts), unless the chip doesn't have it (check the TRM).

trollcop commented 1 year ago

hmm, i think the hardest part of mpp enc would be input frame conversion(with hardware accel), that branch uses legacy RGA, which is broken for newer rockchip chips(with kernel 5.10).

why I cannot find any official repository or librga?

guess so, maybe discuss it in upstream ffmpeg, might be something like !((mode & mask) == deinterlaced) yes that sounds more correct. thank you, i'll mention it upstream.

that should be enabled by default (in kernel defconfig + dts), unless the chip doesn't have it (check the TRM). I'm on firefly AIO_3399J with default ubuntu 20.04 install and whatever default kernel.

So are you saying if I use ffmpeg -i whatever.ts with interlaced mpeg2 it should be auto-deinterlaced as it passes through h264_rkmpp decoder? Or how do I trigger it to happen?

JeffyCN commented 1 year ago

hmm, i think the hardest part of mpp enc would be input frame conversion(with hardware accel), that branch uses legacy RGA, which is broken for newer rockchip chips(with kernel 5.10).

why I cannot find any official repository or librga?

you can check my rockchip-mirrors repo, or 3rd repo (for example firefly), or rockchip's official SDK (needs to buy it). the librga has legacy API and new im2d API, the legacy API is no longer maintained somehow, so it might be broken(at least for the new chips like 3588).

that should be enabled by default (in kernel defconfig + dts), unless the chip doesn't have it (check the TRM). I'm on firefly AIO_3399J with default ubuntu 20.04 install and whatever default kernel.

So are you saying if I use ffmpeg -i whatever.ts with interlaced mpeg2 it should be auto-deinterlaced as it passes through h264_rkmpp decoder? Or how do I trigger it to happen?

that should be enabled by default, you can check: 1/ kernel: config: CONFIG_IEP=y dts: &iep { status = "okay"; };

&iep_mmu { status = "okay"; };

2/ ./mpp/codec/mpp_dec.cpp or mpp/vproc/mpp_dec_vproc.cpp dec->enable_deinterlace <-- should be 1 by default dec->vproc <-- should be non-null dec_vproc_init() <-- should be called successfully dec_vproc_start() <-- should be called successfully

JeffyCN commented 1 year ago

guess so, maybe discuss it in upstream ffmpeg, might be something like !((mode & mask) == deinterlaced) yes that sounds more correct. thank you, i'll mention it upstream.

wait...that doesn't consider mode=0 case. i was using something like this in the gst-mppdec:

  switch (mode & MPP_FRAME_FLAG_FIELD_ORDER_MASK) {
    case MPP_FRAME_FLAG_TOP_FIRST:
      top-first=1;
      /* fall-through */
    case MPP_FRAME_FLAG_BOT_FIRST:
      interlaced=1;
      break;
    default:
      // deinterlaced or normal frame
      break;
  }
trollcop commented 1 year ago

I get feeling none of this stuff is officially supported and is left to the user to experiment.

What happened to gstreamer repository with rockchip patches? it's gone from rockchip-linux github.

My goal is to transcode realtime mpegts in mpegts out, and ffmpeg seemed like the most suitable tool for this.

JeffyCN commented 1 year ago

1/ ffmpeg is never officially supported, it has performance and license issue

2/ that github is kind of dead( due to ffmpeg license issue(pay a lot for it), a bussiness company should never play with ffmpeg:( )

3/ for officially support, needs to by official sdk...

trollcop commented 1 year ago

2/ kind of dead

So where is gstreamer? Does its license work for your company? If so, why is the repository gone? Does it support hardware decode, deinterlace, resize, encode?

3/ for officially support, needs to by official sdk...

We have support contact open with rockchip in taiwan. it's been over a year trying to get various mpp issues fixed and there's still unknown things.

So, with a signed SLA and SDK available, do you have a supported pipeline for: hardware decode, deinterlace, resize, encode?

JeffyCN commented 1 year ago

1/ in official sdk or my github's mirror repo. gone due to ffmpeg license law sue(kind of affraid to open source with official account). yes

2/ you can get a rockchip redmine issue system account to ask it there. from what i know, gst can do this(on 3399). dec/deinterlace/enc based on mpp, hw resize based on legacy rga

due to new rga api, gst hw resize not work for new chips like 3588.

trollcop commented 1 year ago

noted, thanks.

trollcop commented 1 year ago

dts: &iep { status = "okay"; };

&iep_mmu { status = "okay"; };

Well, I think I found the problem.

/proc/device-tree/iep@ff670000/status = disabled.

what is dts and where do I enable this. why is it not enabled out of the box on AIO3399J?