Opened 12 years ago

Closed 12 years ago

#8937 closed defect (fixed)

1080i previews include pixels that are not meant to be active

Reported by: beirdo Owned by: beirdo
Priority: minor Milestone: 0.24
Component: MythTV - General Version: Master Head
Severity: low Keywords:
Cc: Ticket locked: no


1080i is 1920x1080 by definition, but due to H.264 and MPEG-2 macroblock size, etc, it is decoded as 1920x1088. The bottom 8 pixels in the frame are not meant to be active, and should be removed. For playback (and I believe commercial detection), we have dealt with this already, but in generating the previews, we have not.

I have attached a fairly simple patch that will take 1920x1088 frames in the preview generation, and crop off the bottom 8 pixels to recreate 1920x1080 before resizing to the preview size.

Attachments (3)

0001-Fix-green-bar-in-1080i-previews.patch (1.2 KB) - added by beirdo 12 years ago.
preview_1088.diff (532 bytes) - added by markk 12 years ago.
0001-Trim-8-pixels-from-1080i-when-it-s-1088-high-take-2.patch (1.0 KB) - added by beirdo 12 years ago.

Download all attachments as: .zip

Change History (9)

Changed 12 years ago by beirdo

comment:1 Changed 12 years ago by beirdo

Status: newassigned

Changed 12 years ago by markk

Attachment: preview_1088.diff added

comment:2 Changed 12 years ago by anonymous

I've attached an untested, simpler patch that I think should do the trick without the extra image manipulation.

comment:3 Changed 12 years ago by beirdo

The image manipulation is pretty painless, as it doesn't actually copy, it does it in place as best I can tell from the documentation. However, if your patch works and is tested, I'd say go for that, it's certainly less code.

Changed 12 years ago by beirdo

comment:4 Changed 12 years ago by beirdo

OK. Tested markk's patch and updated it slightly to check that it really is 1080i/p resolution with 8 extra rows, not just blindly assume that all 1088 rows are necessarily bad. Good to go as far as I'm concerned. All yours, danielk.

comment:5 Changed 12 years ago by beirdo

Milestone: unknown0.24
Owner: changed from danielk to beirdo
Status: assignedaccepted

Stealing this as danielk mentioned in IRC that he's good with this being added, and told me to go ahead. I will use just a height measurement for the determination as markk mentioned on the development mailing list that there is at least one other offender with 1088 that should be 1080.

comment:6 Changed 12 years ago by beirdo

Resolution: fixed
Status: acceptedclosed

(In [26340]) In the preview generator, if the video's decoded height is 1088, only use the top 1080 pixels to generate the preview. This is done as 1080i H.264 from the HDPVR (and reportedly some other sources) decodes to 1088 rows although 1080i is only 1080 visible rows by definition. Losing these 8 rows removes the ugly green bar at the bottom of these previews, retaining only the visible rows, resized.

Fixes #8937

Note: See TracTickets for help on using tickets.