ConfigReader: normalize the case of enum-like capture options - #971
Open
Cybis320 wants to merge 2 commits into
Open
ConfigReader: normalize the case of enum-like capture options#971Cybis320 wants to merge 2 commits into
Cybis320 wants to merge 2 commits into
Conversation
Contributor
|
Follow-up pushed in bcc412d to address the GStreamer format-casing review finding. Extent of the change:
Validation completed:
@Cybis320, please review this follow-up and test it on your GStreamer/station setup, particularly the |
Contributor
Author
|
Changes look good to me.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Config option names are case-insensitive (
RawConfigParserlowercases keys), but values are read raw, while several are compared against lowercase or uppercase string literals. A value with the wrong case doesn't raise — it silently falls through to the other branch, so the station runs in a mode its config doesn't describe.The clearest example is
protocol:protocol: UDPcaptures over TCP, andObservationSummarystill reportsprotocol_in_useas"UDP"— so the summary disagrees with what GStreamer actually did.Five options are affected. Each is now normalized where it's parsed:
protocol'udp'(BufferedCapture.py).strip().lower()media_backend'gst','v4l2'(BufferedCapture.py).strip().lower()frame_cleanup'delete','tar'(Reprocess.py).strip().lower()frame_file_type'png'(RawFrameSave.py).strip().lower()gst_colorspace'GRAY8'(BufferedCapture.py).strip().upper()gst_colorspacegoes the other way on purpose: it is interpolated into GStreamer caps asformat=..., and those format names are uppercase.Deliberately left alone:
ff_format— used as a literal filename extension, so forcing case would change files on disk.*_file/*_dir/*_pathoptions — filesystem paths, case-sensitive on Linux.shower_color_map,sporadic_color— matplotlib names, case-sensitive there.gst_decoder— passed straight through as a GStreamer element name, never compared to a literal here.Note that section headers (
[Capture],[MeteorDetection], …) remain case-sensitive;has_sectionmatches exactly, so a mis-cased header silently skips the whole section. That's a broader change and isn't touched here.Testing
Parsed the shipped
.configunmodified — values unchanged (tcp,gst,BGR,jpg,delete). Parsed a copy withprotocol: UDP(trailing spaces),media_backend: GST,gst_colorspace: gray8,frame_file_type: PNG,frame_cleanup: Delete— all five resolve to the canonical form the consumers expect.