-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
linux-pipewire: Support MJPEG and H264 devices #11371
base: master
Are you sure you want to change the base?
Conversation
e84c7b6
to
5d60e66
Compare
397bd5a
to
dae65cb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After fighting with the non-specified flow of the source as it is (the format change is done only if the resolution and framerate is changed).
It looks functional as is at least for MJPEG, I can't test H264.
{ | ||
SPA_VIDEO_FORMAT_ENCODED, | ||
DRM_FORMAT_INVALID, | ||
GS_UNKNOWN, | ||
VIDEO_FORMAT_NONE, | ||
false, | ||
0, | ||
"Encoded", | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been thinking about it since a while but I'm not sold at all to put SPA_VIDEO_FORMAT_ENCODER
in the supported_format
array that is mainly meant for non-encoded formats.
Also if the format is encoded, most of the structure member are useless.
I try to make the PR work without this addition.
tytan652@a3092ed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried here dimtpap@910a956
H264 works
The problem is that the format user facing property works with spa_video_format and the distinction between H264 and MJPEG is done with spa_media_subtype
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a idea for that but when I tried to improve the UX with tytan652@ad32134 (similar to #10895 for macOS) which would allow to add a line per codec in your case.
I discovered that, with the upstream code as it is, the format is never enforced and the user can end up with a format not matching the format selected.
In the camera case the format must not be negotiated and follow the user selection.
And PipeWire is not my cup of tea.
if (format != SPA_VIDEO_FORMAT_ENCODED) { | ||
spa_pod_builder_add(b, SPA_FORMAT_mediaSubtype, SPA_POD_Id(SPA_MEDIA_SUBTYPE_raw), 0); | ||
} else { | ||
// TODO: Add EnumFormat for H264 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just marking the TODO
dae65cb
to
a9699a1
Compare
tested this patch on top of gentoo obs-studio-9999 package and it worked for me so far. One little thing: My use case is I can't change resolutions on pipewire-v4l2 so I use obs to change resolution and now video formats. Now I can select 1080p@30fps. I would recommend to test this patch using other software running at the same time. Thank you, now I can go to work meetings using 1080p before it defaults to 640x480. PS: |
Just tested this for my use case in #10180 and it seems to be working very well for MJPEG! Turns out my stream from gphoto2 was raw jpeg and not MJPEG though so I still needed to do some encoding to test sink_name="DSLR"
pipewire_sink_props="p,node.description=$sink_name,node.name=$sink_name,media.role=Camera,media.class=Video/Source"
gphoto2 --stdout --capture-movie \
| gst-launch-1.0 \
fdsrc \
! jpegdec \
! jpegenc \
! jpegparse \
! queue \
! pipewiresink stream-properties="$pipewire_sink_props" |
@dimtpap would you mind resolving the merge conflicts please? |
a9699a1
to
b1edc1c
Compare
For the record I'm waiting for #11741 so I can finish and undraft this |
@dimtpap I've discovered that I sometimes get a segfault when switching between scene collections when a PipeWire MJPEG source is used. Steps to reproduce:
I ran OBS in GDB and reproduced the segfault, here's the log for that: Usually it says |
Description
With #9771, support was added for the Camera portal. However, code to support MJPEG and H264 devices didn't make it. This PR is basically the code that didn't land + some more to finish it up.
Similarly to the v4l2 plugin, it uses ffmpeg to decode the encoded streams.
TODO
enum spa_video_format
. For encoded formats there is only one value of that enum,SPA_VIDEO_FORMAT_ENCODED
and the codec is distinguished usingenum spa_media_subtype
. As of writing the PR actually adds support for just MJPEG because only one EnumFormat is added. I'm unsure as to what is the best approach here.Tried something out here bit it's pretty ugly
enum video_format
with SPAenum spa_video_format
),this caused the formats not to show up in the user selection, but I'm probably wrong since I'm assuming this code worked for as long as it's there? 1, 2. Unfortunately I only have an MJPEG webcam so I couldn't see if it broke other devices.Edit: Tested with a GStreamer pipeline exposing the camera's feed but converted to a raw format. After some more reading it seems this made YUYV422 the only format that resolution and FPS can be selected by the user because the enum values are the same. I don't know if this is intentional or not.
Motivation and Context
MJPEG and H264 devices exist
Fixes #10180
Fixes #10994
How Has This Been Tested?
Connected an MJPEG webcam, added the source, video was coming through.
Types of changes
Checklist: