from:"wallak"

Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API

2017-03-20 Thread wallak



- Mail original -
De: "wm4" 
À: ffmpeg-devel@ffmpeg.org
Envoyé: Lundi 20 Mars 2017 04:23:43
Objet: Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API

On Sun, 19 Mar 2017 11:51:28 -0700
Philip Langdale  wrote:

> On Fri, 17 Mar 2017 19:42:07 -0700
> Philip Langdale  wrote:
> 
> > On Sat, 18 Mar 2017 01:51:39 +0100 (CET)
> > Marton Balint  wrote:
> >   
> > > On Sat, 18 Mar 2017, wal...@free.fr wrote:
> > > 
> > > > The logs:  http://pastebin.com/1duYR0Ui
> > > >  
> > > 
> > > Log with video only (run ffplay with -an -sn) might show it more
> > > clearly, but even from the logs above it looks like the CrystalHD
> > > codec is returning EAGAINs at the same time for both receive_frame
> > > and send_packet. If it indeed does work in ffmpeg, then I suspect a
> > > lot of busy waiting there. Philip, you mind taking a look? :)
> > 
> > My first reply got HTML-ised by my client. Weird.
> > 
> > Wallak,
> > 
> > Can I get a sample from you to ensure I can repro. Also: are you using
> > a 12 or a 15? I only have a 15 and have never tested the code with a
> > 12. Also, it would be great if you can test playback with mpv. As I
> > said, that's what I've used for testing.  
> 
> I did take a brief look, and I confirmed that mpv works fine but ffplay
> is hitting this deadlock between send and receive. My hardware is
> getting flaky, and my actually be dying, so it's hard for me to do any
> kind of systematic testing.
> 
> So right now my only useful advice is to look at how mpv is running the
> loop and see if there's a significant difference.

Currently, mpv pretty strictly does essentially:

while (1) [
  if (!packet)
packet = read_new_packet();
  if (send_packet(packet) == ok)
packet = NULL;
  receive_frame();
}

which I guess is a bit unusual for API usage.

The API should be able to deal with any of that, though.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel



-
On my side, I've checked with a crystalhd BCM970015 (I've a 12 stored 
somewhere). The issue is likely not related to the h264 stream itself.

Wallak.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2017-03-13 Thread wallak

The crystalhd video decoder was not updated to the commit: 
af9cac1be1750ecc0e12c6788a3aeed1f1a778be changes and so is broken.

Is there someone trying to upgrade the crystalhd code? This device is old, but 
is still useful. I hope I'm not the only one using it.

Wallak.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2017-03-13 Thread wallak

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  working without error) is the following one:
234d3cbf469e9feef255e229202d4b029e66e9fe

Is there a configuration flag to fix this issue? A software update is required?

Wallak.

- Mail original -
De: "Philip Langdale" 
À: "FFmpeg development discussions and patches" 
Cc: wal...@free.fr
Envoyé: Mardi 14 Mars 2017 00:13:20
Objet: Re: [FFmpeg-devel] The Crystalhd video decoder is broken - last ffmpeg 
good commit: 6d160afab2fa8d3bfb216fee96d3537ffc9e86e8

On 2017-03-13 07:28, wal...@free.fr wrote:
> The crystalhd video decoder was not updated to the commit:
> af9cac1be1750ecc0e12c6788a3aeed1f1a778be changes and so is broken.
> 
> Is there someone trying to upgrade the crystalhd code? This device is
> old, but is still useful. I hope I'm not the only one using it.
> 
> Wallak.

You're using the latest ffmpeg source? I did this update a long time ago
in 7447ec91b5a692121b81a04c6501a5811d867775

--phil
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2017-03-14 Thread wallak

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" 
À: "Marton Balint" 
Cc: "FFmpeg development discussions and patches" 
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  wrote:

> On Tue, 14 Mar 2017 02:49:27 +0100 (CET)
> wal...@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  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
From 0a221c04973b988fda41d96e4e96f274f8f3caee Mon Sep 17 00:00:00 2001
From: wallak 
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

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2017-03-15 Thread wallak

In the meantime, I have updated the CrystalHD kernel driver to linux 4.9.y: 
https://github.com/wallak/crystalhd

Wallak.

- Mail original -
De: "wm4" 
À: ffmpeg-devel@ffmpeg.org
Envoyé: Mardi 14 Mars 2017 23:35:13
Objet: Re: [FFmpeg-devel] The Crystalhd video decoder is broken - last ffmpeg 
good commit: 6d160afab2fa8d3bfb216fee96d3537ffc9e86e8

On Tue, 14 Mar 2017 22:44:10 +0100 (CET)
Marton Balint  wrote:

> On Mon, 13 Mar 2017, Philip Langdale wrote:
> 
> > On Mon, 13 Mar 2017 19:39:35 -0700
> > Philip Langdale  wrote:
> >  
> >> On Tue, 14 Mar 2017 02:49:27 +0100 (CET)
> >> wal...@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  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?
> >  
> 
> I think I have something working already, yet I did not post it to the 
> list, because I hoped the new API will support subtitles as well, so I 
> wouldn't have to handle them differently...
> 
> I will try to dig up my old patch and submit it.

Subtitles are a whole different complication, so I wouldn't hold my
breath.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API

2017-03-17 Thread wallak

I tried the patch. Once av_assert0 test is removed, the crystalhd decoder is 
running till send_packet triggers AVERROR(EAGAIN), now the crystalhd buffer is 
likely overflowed, the video freezes. A few seconds later the decoder displays 
a few frames and the cycle goes on. 

Im not sure how to properly implement the function to stop or resend 
send_packet frames, this is likely the only mechanism missing.

Best Regards,
Wallak.



- Mail original -
De: "Marton Balint" 
À: "FFmpeg development discussions and patches" 
Cc: wal...@free.fr
Envoyé: Jeudi 16 Mars 2017 23:25:18
Objet: Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API



On Wed, 15 Mar 2017, Marton Balint wrote:

> Since subtitles are not yet supported with the new API, CODEC_CAP_DELAY
> subtitle codecs (only libzvbi so far) may loose the last few buffered frames 
> in
> the end of the stream.
>
> The impact of this is so limited, it seemded better to accept it than losing
> the simplification benefits of the new API.

Hi Wallak,

Have you had a chance to test this ffplay patch, and see if it fixes 
CrystalHD decoding in ffplay or not?

Thanks,
Marton

>
> Signed-off-by: Marton Balint 
> ---
> ffplay.c | 92 +---
> 1 file changed, 41 insertions(+), 51 deletions(-)
>
> diff --git a/ffplay.c b/ffplay.c
> index cf138dc..06d1d22 100644
> --- a/ffplay.c
> +++ b/ffplay.c
> @@ -186,13 +186,10 @@ enum {
> };
> 
> typedef struct Decoder {
> -AVPacket pkt;
> -AVPacket pkt_temp;
> PacketQueue *queue;
> AVCodecContext *avctx;
> int pkt_serial;
> int finished;
> -int packet_pending;
> SDL_cond *empty_queue_cond;
> int64_t start_pts;
> AVRational start_pts_tb;
> @@ -551,40 +548,24 @@ static void decoder_init(Decoder *d, AVCodecContext 
> *avctx, PacketQueue *queue,
> d->queue = queue;
> d->empty_queue_cond = empty_queue_cond;
> d->start_pts = AV_NOPTS_VALUE;
> +d->pkt_serial = -1;
> }
> 
> static int decoder_decode_frame(Decoder *d, AVFrame *frame, AVSubtitle *sub) {
> -int got_frame = 0;
> +int ret = AVERROR(EAGAIN);
> 
> -do {
> -int ret = -1;
> +for (;;) {
> +AVPacket pkt;
> 
> +if (d->queue->serial == d->pkt_serial) {
> +do {
> if (d->queue->abort_request)
> return -1;
> 
> -if (!d->packet_pending || d->queue->serial != d->pkt_serial) {
> -AVPacket pkt;
> -do {
> -if (d->queue->nb_packets == 0)
> -SDL_CondSignal(d->empty_queue_cond);
> -if (packet_queue_get(d->queue, &pkt, 1, &d->pkt_serial) < 0)
> -return -1;
> -if (pkt.data == flush_pkt.data) {
> -avcodec_flush_buffers(d->avctx);
> -d->finished = 0;
> -d->next_pts = d->start_pts;
> -d->next_pts_tb = d->start_pts_tb;
> -}
> -} while (pkt.data == flush_pkt.data || d->queue->serial != 
> d->pkt_serial);
> -av_packet_unref(&d->pkt);
> -d->pkt_temp = d->pkt = pkt;
> -d->packet_pending = 1;
> -}
> -
> switch (d->avctx->codec_type) {
> case AVMEDIA_TYPE_VIDEO:
> -ret = avcodec_decode_video2(d->avctx, frame, &got_frame, 
> &d->pkt_temp);
> -if (got_frame) {
> +ret = avcodec_receive_frame(d->avctx, frame);
> +if (ret >= 0) {
> if (decoder_reorder_pts == -1) {
> frame->pts = 
> av_frame_get_best_effort_timestamp(frame);
> } else if (!decoder_reorder_pts) {
> @@ -593,8 +574,8 @@ static int decoder_decode_frame(Decoder *d, AVFrame 
> *frame, AVSubtitle *sub) {
> }
> break;
> case AVMEDIA_TYPE_AUDIO:
> -ret = avcodec_decode_audio4(d->avctx, frame, &got_frame, 
> &d->pkt_temp);
> -if (got_frame) {
> +ret = avcodec_receive_frame(d->avctx, frame);
> +if (ret >= 0) {
> AVRational tb = (AVRational){1, frame->sample_rate};
> if (frame->pts != AV_NOPTS_VALUE)
> frame->pts = av_rescale_q(frame->pts, 
> av_codec_get_pkt_timebase(d->avctx), tb);
> @@ -606,37 +587,46 @@ static int decoder_decode_frame(Decoder *d, AVFrame 
> *frame, AVSubtitle *sub) {
>

Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API

2017-03-17 Thread wallak

The logs:  http://pastebin.com/1duYR0Ui

Wallak.


- Mail original -
De: "Marton Balint" 
À: "FFmpeg development discussions and patches" 
Cc: wal...@free.fr
Envoyé: Vendredi 17 Mars 2017 23:38:31
Objet: Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API



On Fri, 17 Mar 2017, wal...@free.fr wrote:

> I tried the patch. Once av_assert0 test is removed, the crystalhd 
> decoder is running till send_packet triggers AVERROR(EAGAIN), now the 
> crystalhd buffer is likely overflowed, the video freezes. A few seconds 
> later the decoder displays a few frames and the cycle goes on.
>
> Im not sure how to properly implement the function to stop or resend 
> send_packet frames, this is likely the only mechanism missing.

Hmm. Theoretically an assertion at send_packet can only happen, if the 
CrystalHD decoder is is returning AVERROR(EAGAIN) for receive_frame 
and for send_packet as well, which is forbidden.

Could you add some logging, and log the return values of both send_packet 
and receive_frame to make sure that is the case, and the crystalHD decoder 
is buggy, not my implementation of the new decode API?

I tested h264_cuvid decoder with ffplay (the only decoder with the new 
API), and that worked so I am inclined to think CrystalHD is buggy. And 
the behaviour you described looks like once the buffers of CrystalHD 
is full, but no decoded frame is available yet, it simply returns 
AVERROR(EAGAIN) instead of blocking receive_frame call which it should do.

Thanks,
Marton

> Best Regards,
> Wallak.
>
>
>
> - Mail original -
> De: "Marton Balint" 
> À: "FFmpeg development discussions and patches" 
> Cc: wal...@free.fr
> Envoyé: Jeudi 16 Mars 2017 23:25:18
> Objet: Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API
>
>
>
> On Wed, 15 Mar 2017, Marton Balint wrote:
>
>> Since subtitles are not yet supported with the new API, CODEC_CAP_DELAY
>> subtitle codecs (only libzvbi so far) may loose the last few buffered frames 
>> in
>> the end of the stream.
>>
>> The impact of this is so limited, it seemded better to accept it than losing
>> the simplification benefits of the new API.
>
> Hi Wallak,
>
> Have you had a chance to test this ffplay patch, and see if it fixes 
> CrystalHD decoding in ffplay or not?
>
> Thanks,
> Marton
>
>>
>> Signed-off-by: Marton Balint 
>> ---
>> ffplay.c | 92 
>> +---
>> 1 file changed, 41 insertions(+), 51 deletions(-)
>>
>> diff --git a/ffplay.c b/ffplay.c
>> index cf138dc..06d1d22 100644
>> --- a/ffplay.c
>> +++ b/ffplay.c
>> @@ -186,13 +186,10 @@ enum {
>> };
>> 
>> typedef struct Decoder {
>> -AVPacket pkt;
>> -AVPacket pkt_temp;
>> PacketQueue *queue;
>> AVCodecContext *avctx;
>> int pkt_serial;
>> int finished;
>> -int packet_pending;
>> SDL_cond *empty_queue_cond;
>> int64_t start_pts;
>> AVRational start_pts_tb;
>> @@ -551,40 +548,24 @@ static void decoder_init(Decoder *d, AVCodecContext 
>> *avctx, PacketQueue *queue,
>> d->queue = queue;
>> d->empty_queue_cond = empty_queue_cond;
>> d->start_pts = AV_NOPTS_VALUE;
>> +d->pkt_serial = -1;
>> }
>> 
>> static int decoder_decode_frame(Decoder *d, AVFrame *frame, AVSubtitle *sub) 
>> {
>> -int got_frame = 0;
>> +int ret = AVERROR(EAGAIN);
>> 
>> -do {
>> -int ret = -1;
>> +for (;;) {
>> +AVPacket pkt;
>> 
>> +if (d->queue->serial == d->pkt_serial) {
>> +do {
>> if (d->queue->abort_request)
>> return -1;
>> 
>> -if (!d->packet_pending || d->queue->serial != d->pkt_serial) {
>> -AVPacket pkt;
>> -do {
>> -if (d->queue->nb_packets == 0)
>> -SDL_CondSignal(d->empty_queue_cond);
>> -if (packet_queue_get(d->queue, &pkt, 1, &d->pkt_serial) < 0)
>> -return -1;
>> -if (pkt.data == flush_pkt.data) {
>> -avcodec_flush_buffers(d->avctx);
>> -d->finished = 0;
>> -d->next_pts = d->start_pts;
>> -d->next_pts_tb = d->start_pts_tb;
>> -}
>> -} while (pkt.data == flush_pkt.data || d->queue->serial != 
>> d->pkt_serial);
>> -av_packet_unref(&d

Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API

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

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

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

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

Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API

Re: [FFmpeg-devel] [PATCH 2/3] ffplay: convert to new decode API

7 matches

Site Navigation

Mail list logo

Footer information