diff mbox

[FFmpeg-devel] The Crystalhd video decoder is broken - last ffmpeg good commit: 6d160afab2fa8d3bfb216fee96d3537ffc9e86e8

Message ID 1225176707.239197387.1489524811067.JavaMail.root@zimbra44-e7
State New
Headers show

Commit Message

wallak@free.fr March 14, 2017, 8:53 p.m. UTC 
I've a first patch, working with the software decoder. But it fails with the crystalhd decoder. If you know how to fix it.

Wallak.


----- Mail original -----
De: "Philip Langdale" <philipl@overt.org>
À: "Marton Balint" <cus@passwd.hu>
Cc: "FFmpeg development discussions and patches" <ffmpeg-devel@ffmpeg.org>
Envoyé: Mardi 14 Mars 2017 04:40:34
Objet: Re: [FFmpeg-devel] The Crystalhd video decoder is broken - last ffmpeg good commit: 6d160afab2fa8d3bfb216fee96d3537ffc9e86e8

On Mon, 13 Mar 2017 19:39:35 -0700
Philip Langdale <philipl@overt.org> wrote:

> On Tue, 14 Mar 2017 02:49:27 +0100 (CET)
> wallak@free.fr wrote:
> 
> > Indeed 7447ec91b5a692121b81a04c6501a5811d867775 is working; But I
> > have the following errors with the last ffmpeg git state:
> > [h264_crystalhd @ 0x7fda3c060500] This decoder requires using the
> > avcodec_send_packet() API. Last message repeated 456 times ...
> > 
> > I've 'bisected' this last issue; The last good commit (with ffplay
> > -vcodec h264_crystalhd <file> working without error) is the
> > following one: 234d3cbf469e9feef255e229202d4b029e66e9fe
> > 
> > Is there a configuration flag to fix this issue? A software update
> > is required?  
> 
> Heh - I switched the crystalhd decoder to the new send_packet API in
> the next change, so that's entirely expected. CrystalHD basically
> requires the new API as it allows for flexibility in how often frames
> are returned vs submitted to the decoder; I only ever got it barely
> working with the old API using vicious hacks that failed in many edge
> cases.
> 
> As it uses the new API, the application using the decoder must also
> support the new API. 'ffmpeg' does, but I guess ffplay does not. My
> first reaction is to ask why you're using ffplay. I'd recommend using
> mpv - which is much more capable, and does support the new API; it's
> what I use for all my testing.
> 

Marton,

Would you be interested in updating ffplay to use the new decode API?

--phil

Comments

Philip Langdale March 14, 2017, 9:12 p.m. UTC | #1 
On Tue, 14 Mar 2017 21:53:31 +0100 (CET)
wallak@free.fr wrote:

> I've a first patch, working with the software decoder. But it fails
> with the crystalhd decoder. If you know how to fix it.
> 
> Wallak.

It will take more than that. You need to decouple send and receive -
that's the whole point of the API. Your implementation still assumes
you have a 1:1 relationship - which is true for software decoders but
not for hardware and definitely not for crystalhd.

You could look at the mpv implementation of this for guidance.

> 
> ----- Mail original -----
> De: "Philip Langdale" <philipl@overt.org>
> À: "Marton Balint" <cus@passwd.hu>
> Cc: "FFmpeg development discussions and patches"
> <ffmpeg-devel@ffmpeg.org> Envoyé: Mardi 14 Mars 2017 04:40:34
> Objet: Re: [FFmpeg-devel] The Crystalhd video decoder is broken -
> last ffmpeg good commit: 6d160afab2fa8d3bfb216fee96d3537ffc9e86e8
> 
> On Mon, 13 Mar 2017 19:39:35 -0700
> Philip Langdale <philipl@overt.org> wrote:
> 
> > On Tue, 14 Mar 2017 02:49:27 +0100 (CET)
> > wallak@free.fr wrote:
> > 
> > > Indeed 7447ec91b5a692121b81a04c6501a5811d867775 is working; But I
> > > have the following errors with the last ffmpeg git state:
> > > [h264_crystalhd @ 0x7fda3c060500] This decoder requires using the
> > > avcodec_send_packet() API. Last message repeated 456 times ...
> > > 
> > > I've 'bisected' this last issue; The last good commit (with ffplay
> > > -vcodec h264_crystalhd <file> working without error) is the
> > > following one: 234d3cbf469e9feef255e229202d4b029e66e9fe
> > > 
> > > Is there a configuration flag to fix this issue? A software update
> > > is required?  
> > 
> > Heh - I switched the crystalhd decoder to the new send_packet API in
> > the next change, so that's entirely expected. CrystalHD basically
> > requires the new API as it allows for flexibility in how often
> > frames are returned vs submitted to the decoder; I only ever got it
> > barely working with the old API using vicious hacks that failed in
> > many edge cases.
> > 
> > As it uses the new API, the application using the decoder must also
> > support the new API. 'ffmpeg' does, but I guess ffplay does not. My
> > first reaction is to ask why you're using ffplay. I'd recommend
> > using mpv - which is much more capable, and does support the new
> > API; it's what I use for all my testing.
> > 
> 
> Marton,
> 
> Would you be interested in updating ffplay to use the new decode API?
> 
> --phil
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel




--phil
diff mbox

Patch

From 0a221c04973b988fda41d96e4e96f274f8f3caee Mon Sep 17 00:00:00 2001
From: wallak <wallak@free.fr>
Date: Tue, 14 Mar 2017 21:46:16 +0100
Subject: [PATCH] ffplay: new send_packet API test.

---
 ffplay.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/ffplay.c b/ffplay.c
index cf138dc515..486dc6e26b 100644
--- a/ffplay.c
+++ b/ffplay.c
@@ -583,7 +583,28 @@  static int decoder_decode_frame(Decoder *d, AVFrame *frame, AVSubtitle *sub) {
 
         switch (d->avctx->codec_type) {
             case AVMEDIA_TYPE_VIDEO:
-                ret = avcodec_decode_video2(d->avctx, frame, &got_frame, &d->pkt_temp);
+#define CONFIG_FFMPEGNEWAPISENDPACKET
+#ifdef CONFIG_FFMPEGNEWAPISENDPACKET
+	        {
+		    int decoded = 0;
+		    ret = avcodec_send_packet(d->avctx, &d->pkt_temp);
+		    got_frame = 0;
+		    if (ret >= 0 || ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) {
+		        if (ret >= 0) { decoded = d->pkt_temp.size; }
+			ret = avcodec_receive_frame(d->avctx, frame);
+			if (ret >= 0)
+			  got_frame = 1;
+			if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
+			    ret = 0;
+		    } else {
+		        decoded = d->pkt_temp.size;
+		    }
+		    if (ret >= 0) ret = decoded;
+		}
+#else /*CONFIG_FFMPEGNEWAPISENDPACKET*/
+		ret = avcodec_decode_video2(d->avctx, frame, &got_frame, &d->pkt_temp);
+#endif /*CONFIG_FFMPEGNEWAPISENDPACKET*/
+
                 if (got_frame) {
                     if (decoder_reorder_pts == -1) {
                         frame->pts = av_frame_get_best_effort_timestamp(frame);
@@ -593,7 +614,26 @@  static int decoder_decode_frame(Decoder *d, AVFrame *frame, AVSubtitle *sub) {
                 }
                 break;
             case AVMEDIA_TYPE_AUDIO:
+#ifdef CONFIG_FFMPEGNEWAPISENDPACKET
+	        {
+		    int decoded = 0;
+		    ret = avcodec_send_packet(d->avctx, &d->pkt_temp);
+                    got_frame = 0;
+		    if (ret >= 0 || ret == AVERROR(EAGAIN) || ret == AVERROR_EOF) {
+		      if (ret >= 0) { decoded = d->pkt_temp.size; }
+		      ret = avcodec_receive_frame(d->avctx, frame);
+		      if (ret >= 0)
+			got_frame = 1;
+		      if (ret == AVERROR_EOF)
+			ret = 0;
+		      if (ret == AVERROR(EAGAIN) || ret == AVERROR_EOF)
+			ret = 0;
+		    }
+		    ret = decoded;
+		}
+#else /*CONFIG_FFMPEGNEWAPISENDPACKET*/
                 ret = avcodec_decode_audio4(d->avctx, frame, &got_frame, &d->pkt_temp);
+#endif /*CONFIG_FFMPEGNEWAPISENDPACKET*/
                 if (got_frame) {
                     AVRational tb = (AVRational){1, frame->sample_rate};
                     if (frame->pts != AV_NOPTS_VALUE)
-- 
2.12.0