Skip to content

Gstreamer 1.0: Update to spiir-O4-EW-development

Timothy Davies requested to merge tdavies__catchup_to_O4_dev into gstreamer_python_upgrade

Resolves #56 (closed)

Process

Following !61 (merged) and !62 (merged), the code was caught up to to !24 (merged).

In this MR, I've:

  • Merged from its corresponding merge commit to this branch
    • (no changes were required. There were conflicts between the versions of yapf, with the one used in !62 (merged) putting a newline after a class definition, for example. I undid them since they shouldn't affect anything).
  • Merged from spiir-O4-EW-development into this branch
    • There were a lot of diffs, but not an unmanageable number of conflicts, I don't think it was necessary to break it down into multiple merges. (the intersection between boilerplate gstreamer and python 3 changes/refactors and the things we've been working on for O4 dev isn't too large)
  • Built and ran to catch additional errors
  • Rerun auto formatters

When merging, I read each change in case new gstreamer/Py3 code was being added in O4-dev, or in case old variables/functions were being used in our upgrade. Neither showed up very much, and typically merge conflicts caught it. When building & running, I only found a couple of merges I'd messed up, and one new issue with a python import.

Testing

I've done two runs here: /fred/oz996/tdavies/spiir_project/sources/testing/gout/py3/MR63_tests

The two runs are bit to bit identical to each other and very similar to MR62_tests. My naive vimdiff approach breaks down when comparing to py2 though, it gets the rows out of order. @daniel.tang is considering making a python script to properly compare how different two runs are, so we can better track our progress.

The fact that it builds and runs is good enough for me. I've only had a very brief look for new compiler warnings, but we were already a little flooded. I'm inclined to make a new branch to fix all compiler warnings next, and that should catch anything that might have splipped through.

Review

Merges are a pain to review. For one thing, if I accidentally discarded any changes from O4-dev in this merge it wouldn't show up in the diff. The only way to resolve that is probably to compare this MR against the git diff from O4-dev to !24 (merged). It should be unlikely though.

Besides that, you could read through the diff and make sure everything relates to O4-dev changes? Besides that, there's always testing.

Note only the last 6 commits are relevant (the last 2 were because this branch was build on MR61 and MR62 prior to merge)

Merge request reports