From owner-freebsd-ports-bugs@FreeBSD.ORG Sun Apr 10 12:10:13 2011 Return-Path: Delivered-To: freebsd-ports-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F035B106564A for ; Sun, 10 Apr 2011 12:10:13 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id C5E258FC13 for ; Sun, 10 Apr 2011 12:10:13 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id p3ACADvx035532 for ; Sun, 10 Apr 2011 12:10:13 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id p3ACAD37035531; Sun, 10 Apr 2011 12:10:13 GMT (envelope-from gnats) Date: Sun, 10 Apr 2011 12:10:13 GMT Message-Id: <201104101210.p3ACAD37035531@freefall.freebsd.org> To: freebsd-ports-bugs@FreeBSD.org From: Thomas Zander Cc: Subject: Re: ports/156171: port multimedia/mplayer patch-libao2-ao_oss.c is incorrect X-BeenThere: freebsd-ports-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Thomas Zander List-Id: Ports bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 10 Apr 2011 12:10:14 -0000 The following reply was made to PR ports/156171; it has been noted by GNATS. From: Thomas Zander To: bug-followup@freebsd.org Cc: Subject: Re: ports/156171: port multimedia/mplayer patch-libao2-ao_oss.c is incorrect Date: Sun, 10 Apr 2011 13:46:01 +0200 Hi, while I agree with you that this patch _should_ not be necessary, I am reluctant to simply remove it just now. When you check the cvs log why that patch was included in the first place, http://www.freebsd.org/cgi/cvsweb.cgi/ports/multimedia/mplayer/files/patch-libao2-ao_oss.c the reason was a not-so-obvious problem we had with AC3/DTS passthrough on an SPDIF connection on HDA codecs. The patch fixed (actually worked around) this specific problem without causing harm for anyone else. I think the problem lies deeper also. You are quoting the 4front document about the fact that the CTLs must be set in the order format, channels, sampling rate. But ao_oss.c does not do that. In the non-patched version of reset() it sets, for AC3, first sample rate (line 453), then format. Only for non-AC3 streams it does set in the order format, channels, sample rate and the channel number is set if and only if it's larger than two. This distinction makes the code more convoluted so I presume the original author did this to counter some difficulty. It would make sense to rework this whole section of ao_oss.c to make it compliant with the specs, test the cases, and then get it committed upstream. Until then, we should remove this patch not before we have _tested_ that it does not exhibit regression on the mentioned AC3/DTS/SPDIF/HDA problem. Unfortunately I don't have the hardware at home to test this case. If you do, would you please check? Thanks, Riggs