diff mbox

[FFmpeg-devel] avcodec/rkmpp : Fix broken build and remove some useless code

Message ID MWHPR17MB1614B6FC8CBCC637555A4E2CB41C0@MWHPR17MB1614.namprd17.prod.outlook.com
State Superseded
Headers show

Commit Message

LongChair . Jan. 5, 2018, 7:02 p.m. UTC
Hi,

Here are two patches that should :

- fix https://trac.ffmpeg.org/ticket/6834.

- remove code that looked a bit hackish and which is not necessary anymore.

The first patch is required as mpp removed some control operation from 
the API, breaking the build.

The second one is related to mpp fixes that make things work better.

Both patches have been tested on ROCK64.

More details are included in the patch themselves.

LongChair

Comments

wm4 Jan. 5, 2018, 7:19 p.m. UTC | #1
On Fri, 5 Jan 2018 19:02:25 +0000
"LongChair ." <longchair@hotmail.com> wrote:

> Hi,
> 
> Here are two patches that should :
> 
> - fix https://trac.ffmpeg.org/ticket/6834.
> 
> - remove code that looked a bit hackish and which is not necessary anymore.
> 
> The first patch is required as mpp removed some control operation from 
> the API, breaking the build.
> 
> The second one is related to mpp fixes that make things work better.
> 
> Both patches have been tested on ROCK64.
> 
> More details are included in the patch themselves.

LGTM - I assume the configure changes in the first patch guarantee that
the second patch won't break with old mpp versions, since it raises the
required version high enough.

I can push the patches tomorrow or so, if nobody else has comments.
LongChair . Jan. 5, 2018, 7:22 p.m. UTC | #2
Yes the newly used  control operation seems to have always been there 
anyways, so there shouldn't be much compatibility issues.


On 05/01/2018 20:19, wm4 wrote:
> On Fri, 5 Jan 2018 19:02:25 +0000

> "LongChair ." <longchair@hotmail.com> wrote:

>

>> Hi,

>>

>> Here are two patches that should :

>>

>> - fix https://trac.ffmpeg.org/ticket/6834.

>>

>> - remove code that looked a bit hackish and which is not necessary anymore.

>>

>> The first patch is required as mpp removed some control operation from

>> the API, breaking the build.

>>

>> The second one is related to mpp fixes that make things work better.

>>

>> Both patches have been tested on ROCK64.

>>

>> More details are included in the patch themselves.

> LGTM - I assume the configure changes in the first patch guarantee that

> the second patch won't break with old mpp versions, since it raises the

> required version high enough.

>

> I can push the patches tomorrow or so, if nobody else has comments.

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
wm4 Jan. 5, 2018, 7:30 p.m. UTC | #3
On Fri, 5 Jan 2018 19:22:00 +0000
"LongChair ." <longchair@hotmail.com> wrote:

> Yes the newly used  control operation seems to have always been there 
> anyways, so there shouldn't be much compatibility issues.

I mean the second patch removes a workaround for some old misbehavior,
right? So it should probably be made sure that the user can't use an
old version.

Also please avoid https://en.wikipedia.org/wiki/Top_posting#Top-posting
on the list.
LongChair . Jan. 5, 2018, 7:51 p.m. UTC | #4
Yes this was bound to very old versions afaik.

Mpp repo was squashed and we didn't have any version requirement

Current version is 1.3.7, if i trust my repo fork, previous version was 
1.0.0, so we could add that as a requirement.


On 05/01/2018 20:30, wm4 wrote:
> On Fri, 5 Jan 2018 19:22:00 +0000

> "LongChair ." <longchair@hotmail.com> wrote:

>

>> Yes the newly used  control operation seems to have always been there

>> anyways, so there shouldn't be much compatibility issues.

> I mean the second patch removes a workaround for some old misbehavior,

> right? So it should probably be made sure that the user can't use an

> old version.

>

> Also please avoid https://en.wikipedia.org/wiki/Top_posting#Top-posting

> on the list.

> _______________________________________________

> ffmpeg-devel mailing list

> ffmpeg-devel@ffmpeg.org

> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
diff mbox

Patch

From 46523ce0fca740a69619e59398d329623800e501 Mon Sep 17 00:00:00 2001
From: LongChair <longchair@hotmail.com>
Date: Tue, 2 Jan 2018 12:38:01 +0100
Subject: [PATCH] avcodec/rkmpp : remove stream start retries before first
 frame.

those were needed because of some odd mpp behavior that seems to have been fixed.
Makes the code cleaner.
---
 libavcodec/rkmppdec.c | 24 +++---------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/libavcodec/rkmppdec.c b/libavcodec/rkmppdec.c
index 946b827918..143d05bd51 100644
--- a/libavcodec/rkmppdec.c
+++ b/libavcodec/rkmppdec.c
@@ -47,7 +47,6 @@  typedef struct {
     MppApi *mpi;
     MppBufferGroup frame_group;
 
-    char first_frame;
     char first_packet;
     char eos_reached;
 
@@ -329,28 +328,14 @@  static int rkmpp_retrieve_frame(AVCodecContext *avctx, AVFrame *frame)
     MppBuffer buffer = NULL;
     AVDRMFrameDescriptor *desc = NULL;
     AVDRMLayerDescriptor *layer = NULL;
-    int retrycount = 0;
     int mode;
     MppFrameFormat mppformat;
     uint32_t drmformat;
 
-    // on start of decoding, MPP can return -1, which is supposed to be expected
-    // this is due to some internal MPP init which is not completed, that will
-    // only happen in the first few frames queries, but should not be interpreted
-    // as an error, Therefore we need to retry a couple times when we get -1
-    // in order to let it time to complete it's init, then we sleep a bit between retries.
-retry_get_frame:
     ret = decoder->mpi->decode_get_frame(decoder->ctx, &mppframe);
-    if (ret != MPP_OK && ret != MPP_ERR_TIMEOUT && !decoder->first_frame) {
-        if (retrycount < 5) {
-            av_log(avctx, AV_LOG_DEBUG, "Failed to get a frame, retrying (code = %d, retrycount = %d)\n", ret, retrycount);
-            usleep(10000);
-            retrycount++;
-            goto retry_get_frame;
-        } else {
-            av_log(avctx, AV_LOG_ERROR, "Failed to get a frame from MPP (code = %d)\n", ret);
-            goto fail;
-        }
+    if (ret != MPP_OK && ret != MPP_ERR_TIMEOUT) {
+        av_log(avctx, AV_LOG_ERROR, "Failed to get a frame from MPP (code = %d)\n", ret);
+        goto fail;
     }
 
     if (mppframe) {
@@ -366,7 +351,6 @@  retry_get_frame:
             avctx->height = mpp_frame_get_height(mppframe);
 
             decoder->mpi->control(decoder->ctx, MPP_DEC_SET_INFO_CHANGE_READY, NULL);
-            decoder->first_frame = 1;
 
             av_buffer_unref(&decoder->frames_ref);
 
@@ -480,7 +464,6 @@  retry_get_frame:
                 goto fail;
             }
 
-            decoder->first_frame = 0;
             return 0;
         } else {
             av_log(avctx, AV_LOG_ERROR, "Failed to retrieve the frame buffer, frame is dropped (code = %d)\n", ret);
@@ -560,7 +543,6 @@  static void rkmpp_flush(AVCodecContext *avctx)
 
     ret = decoder->mpi->reset(decoder->ctx);
     if (ret == MPP_OK) {
-        decoder->first_frame = 1;
         decoder->first_packet = 1;
     } else
         av_log(avctx, AV_LOG_ERROR, "Failed to reset MPI (code = %d)\n", ret);
-- 
2.14.1