After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 642691 - deinterlace: Miscellaneous cleanup
deinterlace: Miscellaneous cleanup
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: 0.10.29
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2011-02-18 17:04 UTC by Robert Swain
Modified: 2011-03-08 15:32 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
deinterlace: Fix assigned method_id when using a fallback (2.55 KB, patch)
2011-02-18 17:16 UTC, Robert Swain
committed Details | Review
deinterlace: Port greedyl to GstDeinterlaceSimpleMethod (13.29 KB, patch)
2011-02-18 17:16 UTC, Robert Swain
reviewed Details | Review
deinterlace: Simplify setcaps (7.80 KB, patch)
2011-02-18 17:17 UTC, Robert Swain
committed Details | Review
deinterlace: Port greedyl to GstDeinterlaceSimpleMethod (12.58 KB, patch)
2011-02-22 14:59 UTC, Robert Swain
accepted-commit_now Details | Review
deinterlace: Port greedyl to GstDeinterlaceSimpleMethod (13.16 KB, patch)
2011-02-23 12:12 UTC, Robert Swain
committed Details | Review

Description Robert Swain 2011-02-18 17:04:25 UTC
Some patches follow...
Comment 1 Robert Swain 2011-02-18 17:16:03 UTC
Created attachment 181243 [details] [review]
deinterlace: Fix assigned method_id when using a fallback

    Also improve debug output by printing the buffer pointer when
    popping a buffer and simplify code to use scanlines.bottom_field as
    appropriate.
Comment 2 Robert Swain 2011-02-18 17:16:50 UTC
Created attachment 181245 [details] [review]
deinterlace: Port greedyl to GstDeinterlaceSimpleMethod

    The main goal of this change is to reuse the complex but now neatly
    written scanline pointer calculation code from the simple methods.
Comment 3 Robert Swain 2011-02-18 17:17:24 UTC
Created attachment 181246 [details] [review]
deinterlace: Simplify setcaps

    The current code never uses upstream negotiation so the code can be
    significantly simplified.
Comment 4 Tim-Philipp Müller 2011-02-21 20:30:48 UTC
Comment on attachment 181245 [details] [review]
deinterlace: Port greedyl to GstDeinterlaceSimpleMethod

Does this actually work for you?

For me, it looks like it just doesn't do any interlacing any more with method=greedyl
Comment 5 Tim-Philipp Müller 2011-02-21 20:35:59 UTC
> For me, it looks like it just doesn't do any interlacing any more with
> method=greedyl

Doesn't do any *de*interlacing I meant of course.
Comment 6 Robert Swain 2011-02-21 20:51:23 UTC
Well it certainly worked in my tree and I tested it. Did you apply all the above patches or just that one? I think that perhaps the method_id fix was related to that greedyl porting and not to my other work.
Comment 7 Tim-Philipp Müller 2011-02-21 23:21:48 UTC
I applied the method_id fix and the greedyl patch, but not the setcaps patch.

Here's one of the videos I've been testing with:

http://people.freedesktop.org/~tpm/samples/bbcnews2.m2t

It's really easy to see if you look at the scrolling text at the bottom.
Comment 8 Robert Swain 2011-02-22 14:59:58 UTC
Created attachment 181590 [details] [review]
deinterlace: Port greedyl to GstDeinterlaceSimpleMethod

Fix copied lines.
Comment 9 Robert Swain 2011-02-22 15:00:50 UTC
(In reply to comment #7)
> I applied the method_id fix and the greedyl patch, but not the setcaps patch.
> 
> Here's one of the videos I've been testing with:
> 
> http://people.freedesktop.org/~tpm/samples/bbcnews2.m2t
> 
> It's really easy to see if you look at the scrolling text at the bottom.

Try the new version please. :) I don't know why I did it like that. Perhaps there was something wrong with using m2 in another case while I was adjusting all the code. I'm not sure.
Comment 10 Tim-Philipp Müller 2011-02-22 15:18:43 UTC
Comment on attachment 181590 [details] [review]
deinterlace: Port greedyl to GstDeinterlaceSimpleMethod

Works for me now as well. 

There's still some vertical instability - look at the red text on the white background, it looks like it's jittering up and down (hard to see with the fast movements in the main picture), but that's been there in the old method as well, so not related to this patch.
Comment 11 Robert Swain 2011-02-23 12:12:02 UTC
Created attachment 181687 [details] [review]
deinterlace: Port greedyl to GstDeinterlaceSimpleMethod

This update should fix crashes being observed with the obsoleted patches. Previously the interpolation function was being run always using the stride from the Y plane, regardless of the plane being operated on. This didn't break anything previously because the bottom lines were special-cased to copy and the copy functions used the correct strides.
Comment 12 Robert Swain 2011-02-23 12:27:32 UTC
Comment on attachment 181687 [details] [review]
deinterlace: Port greedyl to GstDeinterlaceSimpleMethod

Accepted on IRC
Comment 13 Robert Swain 2011-02-23 12:28:00 UTC
Comment on attachment 181246 [details] [review]
deinterlace: Simplify setcaps

Accepted on IRC
Comment 14 Robert Swain 2011-02-23 12:28:26 UTC
commit 6607cdcc08621c99ad96be4d7c57c92674d7c785
Author: Robert Swain <robert.swain@collabora.co.uk>
Date:   Mon Nov 8 14:25:59 2010 +0100

    deinterlace: Simplify setcaps
    
    The current code never uses upstream negotiation so the code can be
    significantly simplified.

commit 6402556157c8a1f9da6eae421b426293f6f5039c
Author: Robert Swain <robert.swain@collabora.co.uk>
Date:   Mon Jan 24 12:48:18 2011 +0100

    deinterlace: Port greedyl to GstDeinterlaceSimpleMethod
    
    The main goal of this change is to reuse the complex but now neatly
    written scanline pointer calculation code from the simple methods.