Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Michael Niedermayer <michael@niedermayer.cc>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [RFC] SDR
Date: Thu, 22 Jun 2023 19:43:11 +0200
Message-ID: <20230622174311.GE3250409@pb2> (raw)
In-Reply-To: <168745018738.21886.10926077684605560333@lain.khirnov.net>


[-- Attachment #1.1: Type: text/plain, Size: 2669 bytes --]

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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2023-06-22 17:43 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-22 14:57 Michael Niedermayer
2023-06-22 15:01 ` James Almer
2023-06-22 15:09   ` Thilo Borgmann
2023-06-22 16:09 ` Anton Khirnov
2023-06-22 17:43   ` Michael Niedermayer [this message]
2023-06-22 18:52     ` Michael Niedermayer
2023-06-23 11:26   ` James Almer
2023-06-23 20:18 ` Tomas Härdin
2023-06-23 20:19   ` Tomas Härdin
2023-06-23 21:36     ` Michael Niedermayer
2023-06-24 10:12       ` Tomas Härdin
2023-06-24 21:29         ` Michael Niedermayer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230622174311.GE3250409@pb2 \
    --to=michael@niedermayer.cc \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git