[FFmpeg-devel] [RFC] SDR

[Michael Niedermayer]

On Thu, Jun 22, 2023 at 06:09:47PM +0200, Anton Khirnov wrote:
> Hi,
> I'm sorry to say, the patch at its current state is quite far from
> something I'd consider acceptable for git master.
> 
> 1) Given that there are sdr and sdrfile demuxers, it seems to me that
> the transport layer should be split into a protocol rather than be
> inside the demuxer

sdrfile is for debuging and testing

can sdrdemux be split into a protocol and demuxer. Probably yes.
This will make the code more difficult to maintain, more complex and and
maybe slower.
Also keep in mind Soapy has some "zero copy" support that i have not looked
into yet. I doubt that becomes easier if it has to be funnled through an extra
layer

And other devices do not do that, alsa, oss, video4linux have no seperate
protocol.
and adding a alsafile or v4lfile debug option at least to me does not seem
like a good reason to redesign these with protocols
Or consider a alsa or video4linux fate test that used a file as input
It makes sense for these too to have such file choice


> 2) Threading inside the demuxer, with pthread calls randomly sprinkled
> throughout is particularly ugly and dangerous. Demuxers should not do
> their own threading.

IIRC the only reason why any threading is needed ATM is because the buffers
from soapy are too small with some hardware.

i dont think its fair to call the pthread code randomly sprinkled when
all mutex code is around either fifo calls or the seek direction or wanted frequency
fields.
In fact if AVFifo was thread safe half the lock/unlock calls would be gone

Of course in the future we want to do the FFT in 2 or more threads so the
maximum that can be processed is higher (some expensive hw can deliver
alot more data)

But more important than "ugly" is the dangerous bit, can you elaborate
what is dangerous here, especially in relation to how to make it better ?


> 3) sdr_read_header() and sdr_read_packet() are giant, hideous, and
> unreadable.

I disagree that they are hideous and unreadable. In fact its largely just
setting up the hw in sdr_read_header() and thats very close to what the
libsoapy example code does.

But ill see if it can be split a bit.


> 4) Why does there seem to be drawing code in a demuxer?

Because in a practical use case the user needs feedback from what is
available in the radio spectrum. That can be disabled, ill
change this to be disabled by default.

Thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20230622/c722b10a/attachment.sig>